Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add exports field to support newer versions of nodejs #206

Open
jamie-pate opened this issue Feb 11, 2022 · 3 comments · May be fixed by #207
Open

Add exports field to support newer versions of nodejs #206

jamie-pate opened this issue Feb 11, 2022 · 3 comments · May be fixed by #207

Comments

@jamie-pate
Copy link

jamie-pate commented Feb 11, 2022

nodejs ignores the 'module' field of package.json and only supports the 'exports' field (which is much more vesatile) if your package is "type": "module"

Please add the exports field to allow nodejs to load the es5m build instead of the es5 build, otherwise e.g. TypeScript with allowSyntheticDefaultImports gets confused and creates 'default' imports from the es5 build instead.

import makeWebsocketObservable from 'rxjs-websocket';

console.log(makeWebsocketObservable);

{
normalClosureMessage: 'Normal closure',
default: [Function: makeWebSocketObservable]
}

(should be [Function: makeWebsocketObservable])

see
https://github.com/nodejs/node/blob/v16.14.0/lib/internal/modules/esm/resolve.js#L911 where the nodejs module importer checks for the exports field in the packageConfig (package.json) and will call legacyMainResolve() if it doesn't exist (only checks the package.json main field)
https://github.com/nodejs/node/blob/v16.14.0/lib/internal/modules/esm/resolve.js#L299

@jamie-pate
Copy link
Author

workaround:

import { default as mwo, WebSocketFactory, WebSocketLike } from 'rxjs-websockets';
const makeWebsocketObservable = typeof mwo === 'function' ? mwo : (mwo as unknown as {default: typeof mwo}).default;

@insidewhy
Copy link
Owner

Cool thanks for prompting me on this. You wanna open a PR?

@jamie-pate
Copy link
Author

jamie-pate commented Feb 12, 2022

ok, so the output file also needs to be renamed to '.mjs' because reasons ... how do you feel about "build-es5m": "yarn build-es5 -m es2015 --outDir dist.es5m; mv index.js index.mjs" which would break any cross platform support for building outside of a unix environment? 🤦

.. what a mess https://www.typescriptlang.org/docs/handbook/esm-node.html

jamie-pate added a commit to jamie-pate/rxjs-websockets that referenced this issue Feb 12, 2022
…sers

Fixes insidewhy#206
This ensures that consumers using --experimental-modules with node 12+
will import the esm version of this package.

This package's export MUST have .mjs file extension... (because
otherwise the entire package must be specified as type: module)
This package's export MUST use exports field in the package.json
	see https://nodejs.org/api/packages.html#conditional-exports

Not sure how to test this with a bunch of different project styles..
webpack and other loaders _should_ be fine with importing from the
`module` field with a `.mjs` file extension?
jamie-pate added a commit to jamie-pate/rxjs-websockets that referenced this issue Feb 14, 2022
…sers

Fixes insidewhy#206
This ensures that consumers using --experimental-modules with node 12+
will import the esm version of this package.

This package's export must a package.json with "type": "json" (or module
files must have .mjs extension)...
This package's export must also use exports field in the package.json
because node doesn't support the "module" field as an alternative to
"main".
	see https://nodejs.org/api/packages.html#conditional-exports
jamie-pate added a commit to jamie-pate/rxjs-websockets that referenced this issue Feb 14, 2022
…sers

Fixes insidewhy#206
This ensures that consumers using --experimental-modules with node 12+
will import the esm version of this package.

This package's export must a package.json with "type": "json" (or module
files must have .mjs extension)...
This package's export must also use exports field in the package.json
because node doesn't support the "module" field as an alternative to
"main".
	see https://nodejs.org/api/packages.html#conditional-exports
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants