build: support for webpack and editors

[peterpeterparker]

Motivation

It was reported that the library does not work with Next.JS but, we were not abole to reproduce the issue. However, the developer is using a custom Webpack, that's why we can actually assume that the issue related to this bundle.

On another side, the issue does not seems to reside solely in the build but, also when imported in an editor as Webstorm or VS Code does not seem able to understand the lib is modular.

That's why, there are various things we should do to fix the problem.

Webpack

Webpack seems to require a sideEffects flag in package.json to enable tree shaking for a library (documentation). This was, for example, introduced in three.js.

I'm also unsure whether Webpack requires a main field. This PR adds the main entry, even though it will still resolve to an index that throws an error, since we are providing a modular library.

Import and validation

When using exports in package.json it seems that editor have troube understanding that the modules are not shipped at the root of the library. I did not find a solution therefore decided to adapt the way we provide the information by moving package.json, and other metadata filesto thedist` folder. That way the package can reference files at the same level.

⚠️ This comes with an important change. The library after npm run build should not be published anymore from the root of the library but, from the dist folder. That is why the pipelines had to be updated.

Other changes

While fixing those issues I noticed few other minor problems. The mocks.ts was packaged in the lib and the file LICENSE was missing.

[lmuntaner]

Thanks for the research and changes!

However, I'm not sure about publishing from the dist directory. It seems a brittle thing easy to forget or ignore. I think that I prefer to drop the index raising an error instead.

I never had a problem with VS Code identifying the import. Do you have it in Webstorm in Oisy?

This problem doesn't seem to happen in the canary version of NextJS when I tested it. Therefore, I think it's safe to assume it won't happen in new NextJS versions.

Removing the throwin an error in the index simplifies things.

[lmuntaner]

There is already a LICENSE in the root directory. Why is this also needed here?

[peterpeterparker]

However, I'm not sure about publishing from the dist directory. It seems a brittle thing easy to forget or ignore. I think that I prefer to drop the index raising an error instead.

As far as I know, it's also common practice to release a flattened library "from the dist directory", such as in Angular libraries or noble-hashes, which we used as inspiration to componse this library.

Do you have it in Webstorm in Oisy?

I have a similar issue in WebStorm, like the one reported in issue #48.

This problem doesn't seem to happen in the canary version of NextJS when I tested it.

Did you test it using the reporter's project or with a sample repository?

The issue is not with Next.js, but with webpack. I couldn't reproduce the issue with a sample Next.js repository either.

Removing the throwin an error in the index simplifies things.

This might work as a workaround fix for this library, but in my opinion, it's not a long-term solution for the foundation, especially considering we have other projects like the Oisy Signer library, which aim to be modular without requiring multiple libraries to be published on npm. Additionally, this approach would limit the library to being a single entity, restricting future extensions or additions.

There is already a LICENSE in the root directory. Why is this also needed here?

The LICENSE in the root directory is not included when the library is shipped. You can check the node_modules folder or the "Code" tab on npm to verify this.

[peterpeterparker]

Note that if your concern is publishing from the js-directory folder, we can add a prepublishOnly hook or set the private: true field to prevent this from happening. I think it's a valid concern and we can/should do that.

[lmuntaner]

As far as I know, it's also common practice to release a flattened library "from the dist directory", such as in Angular libraries or noble-hashes, which we used as inspiration to componse this library.

Where do you see noble/hashes publishing from the build directory? Their publish action seems to run from the root directory: https://github.com/paulmillr/noble-hashes/blob/main/.github/workflows/publish-npm.yml#L20

I'm not familia with this practice and I'm afraid it can lead to unknown issues in the future. That's why I'm skeptical.

But if you say it's a common practice it's ok for me. That probably means there are not many unknowns ahead.

[peterpeterparker]

Where do you see noble/hashes publishing from the build directory?

Right, they build to the root directory.

They generate JS files at the root: https://github.com/paulmillr/noble-hashes/blob/f51bab49ec460e227706ffcc1eeec85db31053aa/tsconfig.json#L5

They clean the root directory before build: https://github.com/paulmillr/noble-hashes/blob/f51bab49ec460e227706ffcc1eeec85db31053aa/package.json#L17

They also ignore JS files at the root: https://github.com/paulmillr/noble-hashes/blob/f51bab49ec460e227706ffcc1eeec85db31053aa/.gitignore#L4

As a result, the library they ship also contains the src folder.

Output: https://www.npmjs.com/package/@noble/hashes?activeTab=code

Shipping from dist does sound familiar to me and cleaner, I rather like when I develop locally to not have those bundled files in the root of my environment, but if you prefer an approach where it builds to the root, I’m not opposed. Ultimately, for the Oisy Signer (and in general), I think we should move towards a more modular approach, happy to adapt the rest.

Let me know WDYT!

[lmuntaner]

Shipping from dist seems cleaner than all that, I agree.

Interesting that they build twice with different tsconfigs: tsc & tsc -p tsconfig.esm.json

The esm tsconfig: https://github.com/paulmillr/jsbt/blob/main/tsconfig.esm.json

the other one: https://github.com/paulmillr/jsbt/blob/main/tsconfig.cjs.json

I wonder whether we should do something similar? Not in this PR though.

[peterpeterparker]

wonder whether we should do something similar?

If I remember correctly you told me this library only works in the browser? If that's correct, we do not need that approach. On the contrary sure we can also ship for Node but, i would ship esm as well (event if Node is going to support cjs again soon, I guess they gonna keep esm as default, we can double check).

[peterpeterparker]

We also discussed this offline.

[peterpeterparker]

Reporter of the issue confirmed this PR resolved the issue (see comment). Therefore we merge it.