chore: set tsconfig moduleResolution to Node16

[ocavue]

1. Set the moduleResolution as Node16, so that tsc -p tsconfig.esm.json would warn us with the following error for missing file extension like the one in Downstream builds failing due to ambiguous import #24.

src/utils.ts:2:42 - error TS2835: Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean './_assert.js'?

2 import { bytes as abytes, isBytes } from './_assert';

2. Replaced import ... from '@noble/ciphers/crypto'; to import ... from './crypto.js';. Not sure why it was with the full package name.

3. Added ./crypto.js": "./esm/cryptoNode.js" in esm/package.json so that it will target the correct file in Node.js environment. I'm not sure if this is necessary but it seems to be harmless to add it.
   In my opinion, the clearer way is having four different builds: (1) CommonJS (using globalThis.crypto); (2) ES2020 (using globalThis.crypto); (3) CommonJS_Node (using node:crypto); (4) ES2020_Node (using node:crypto). In this way, we can remove the "browser" and "node" fileds in package.json, and have something like this in the package.json:

  "exports": {
    "./aes": {
      "types": "./dist/types/aes.d.ts",
      "node": {
        "import": "./dist/node_esm/aes.js",
        "default": "./dist/node_cjs/aes.js"
      },
      "import": "./dist/esm/aes.js",
      "default": "./dist/cjs/aes.js"
    }
  }

Since we only use exports (and don't use top-level node nor browser) for different builds, this should have the best compatibility.
I can implement this structure if you agree.

[paulmillr]

the current crypto import behavior was copied from noble-hashes. I don’t remember why it’s this way. There’s probably some issue in some bundler (webpack, rollup, browserify, esbuild).

If it works, then we shouldn’t touch it. Having 4 versions is not acceptable: it becomes much harder to audit and inflates bundle size a lot. When node v18 is deprecated, we would just fully switch to webcrypto.

Also tests are failing.

[ocavue]

If it works, then we shouldn’t touch it.

I agree with this strategy.

Also tests are failing.

Fixed.

[paulmillr]

we don’t need src/package.json. we already have lib/esm/package.json

[ocavue]

tsc cannot detect incorrect import ... from './_assert'; (without .js extension) if we remove src/package.json.

$ cat src/utils.ts | grep _assert
import { bytes as abytes, isBytes } from './_assert';

$ npm run build

> @noble/[email protected] build
> npm run build:clean; tsc && tsc -p tsconfig.esm.json


> @noble/[email protected] build:clean
> rm *.{js,d.ts,js.map,d.ts.map} esm/*.{js,d.ts,js.map,d.ts.map} 2> /dev/null

src/utils.ts:2:42 - error TS2835: Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean './_assert.js'?

2 import { bytes as abytes, isBytes } from './_assert';
                                           ~~~~~~~~~~~


Found 1 error in src/utils.ts:2


$ rm src/package.json
$ npm run build

> @noble/[email protected] build
> npm run build:clean; tsc && tsc -p tsconfig.esm.json


> @noble/[email protected] build:clean
> rm *.{js,d.ts,js.map,d.ts.map} esm/*.{js,d.ts,js.map,d.ts.map} 2> /dev/null

$ 

[paulmillr]

tsc cannot detect incorrect import ... from './_assert'; (without .js extension) if we remove src/package.json.

Yes, it should not detect it, that's fine. I use extensions for all files inside of the project except for crypto. Outside of the project, there is no need for extensions, since we have import maps in package.json.

[paulmillr]

ah, that was the whole reason for the pull request; for us to be able to detect these bugs

[paulmillr]

Changing module resolution from bundler to node16 could ruin the build process for webpack / rollup / esbuild. It needs to be tested as well.

There was this gist: https://gist.github.com/paulmillr/cf5f8441c93dfef87c814af18a06d166 which should be re-evaluated, but probably a couple of other issues.

[ocavue]

that was the whole reason for the pull request; for us to be able to detect these bugs

Exactly!

Changing module resolution from bundler to node16 could ruin the build process for webpack / rollup / esbuild.

This commit shows the diff of built JS files before and after this PR. Except for comment changes, only esm/index.js is changed. And I don't think that change would break anything.

1fc2c4a

[paulmillr]

This commit shows the diff of built JS files before and after this PR. Except for comment changes, only esm/index.js is changed. And I don't think that change would break anything.

That's false, i've just copied ciphers into ciphers-16 and changed moduleResolution and module to node16. The result:

noble ❯ diff ciphers ciphers-16 | wc -l
2400

The changes are quite big. For example, it changes all export a into exports.a:

-export const cbc = generate("AES-CBC" /* BlockMode.CBC */);
-export const ctr = generate("AES-CTR" /* BlockMode.CTR */);
-export const gcm = generate("AES-GCM" /* BlockMode.GCM */);
+exports.cbc = generate("AES-CBC" /* BlockMode.CBC */);
+exports.ctr = generate("AES-CTR" /* BlockMode.CTR */);
+exports.gcm = generate("AES-GCM" /* BlockMode.GCM */);

[paulmillr]

Nevermind: i've forgot to copy the src/package.json. All good. I have found a way to not do ts-ignore though.

[ocavue]

I have found a way to not do ts-ignore though.

Oh your method is great!

[paulmillr]

I wish we could remove this @noble/ciphers/crypto import altogether though and replace it with ./crypto.js. Don't know how to do it right now.

[ocavue]

I wish we could remove this @noble/ciphers/crypto import.

Having 4 versions is not acceptable: it becomes much harder to audit and inflates bundle size a lot.

One method would be moving crypto.ts and cryptoNode.ts into a new NPM package, let's say @noble/crypto, and make this new package have 4 versions (esm.js, cjs.js, node_esm.js, node_cjs.js). Then we can let @crypto/ciphers depend on this new package, and get rid of all kinds of magics like @noble/ciphers/crypto import or the node top-level fied in package.json. @crypto/ciphers still only has two versions (esm and cjs). This won't inflate the bundle size.

However, this does break the "zero dependency" rule, which is a downside.

[paulmillr]

I don’t think this is a long term solution.

For example, dependencies won’t really work in deno. They would only work if npm specifier is utilized.

If we just drop this conditional altogether once node v18 is deprecated, that would work much better.

[ocavue]

dependencies won’t really work in deno. They would only work if npm specifier is utilized.

Good point