feat: esm support #161
feat: esm support #161
Conversation
|
After digging more, for Node.js to be compatible the file extension of the modules must be Since React does not ship ESM modules Node.js support does not matter that much, however, this library will hopefully not require any more changes when react starts shipping esm modules :) |
|
hi @n1ru4l, |
8085749 to
cb1f404
Compare
|
@morrys done :) |
|
@n1ru4l I tried the bundle in the example projects cra and pagination-nextjs-ssr and I get errors similar to this: facebook/create-react-app#10356 |
|
@morrys Seems like create-react-app is not properly configured to support the An alternative approach (that could potentially break the common js module) could be the following: https://unpkg.com/browse/@tinyhttp/app@1.1.12/package.json "type": "module",This tells Node.js that any |
|
I found this and will try it: d3/d3#3469 (comment) |
|
When will this be merged? |
|
For now it is not merged because there are problems #161 (comment) |
|
i now have a solution that works consistently. I will try to update the PR this week :) |
The logic for this is super simple to inline and does not require an external dependency to achieve.
| format === "cjs" && { | ||
| name: "writePkgJSON", | ||
| writeBundle: async () => { | ||
| await fs.writeFile( | ||
| "lib/cjs/package.json", | ||
| JSON.stringify({ | ||
| type: "commonjs", | ||
| }) | ||
| ); | ||
| } | ||
| }, | ||
| format === "es" && { | ||
| name: "writePkgJSON", | ||
| writeBundle: async () => { | ||
| await fs.writeFile( | ||
| "lib/package.json", | ||
| JSON.stringify({ | ||
| type: "module", | ||
| }) | ||
| ); | ||
| } | ||
| }, |
There was a problem hiding this comment.
A lib/package.json with the contents {"type":"module"} and also a lib/cjs/package.json with the contents {"type":"commonjs"} is generated.
This tells Node.js to treat the contents of that folder/subfolder as the given file type without having to specify the .cjs or .mjs extension. I did not specify it in the root package.json as it caused issues with babel and jest (jest is not es ready yet).
There was a problem hiding this comment.
I also noticed that the lib folder also includes the "raw" js files besides the bundles. Do we really need those raw files? Why would one want to import those directly instead of using the top-level exports, do you have any use-case in mind? Would it be enough to simply have the typescript definition files and the bundled rollup code?
| "exports": { | ||
| ".": { | ||
| "import": "./lib/es-relay-hooks.mjs", | ||
| "require": "./lib/cjs/cjs-relay-hooks.js" | ||
| }, | ||
| "./package.json": "./package.json" | ||
| }, |
There was a problem hiding this comment.
Starting with Node.js 14 you cannot simply import anything from a package. You can only use the exports declared in the exports map.
E.g. import { fetchResolver } from "relay-hooks/lib/FetchResolver"; would fail. If that should be possible a entry for each file must be declared in the exports map.
There was a problem hiding this comment.
hi @n1ru4l,
i tried this package in react-relay-offline and i get this error.
react-relay-offline\node_modules\relay-hooks\lib\QueryFetcher.js:1
({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){import * as areEqual from 'fbjs/lib/areEqual';
SyntaxError: Cannot use import statement outside a module
Did you mean this?
There was a problem hiding this comment.
If you are referring to this: https://github.com/morrys/react-relay-offline/blob/c12787d3d0c3325389cd4c8d06ea2f3ffdac800a/src/runtime/loadQuery.ts#L3
Then yes, instead this file should be explicitly exposed via the exports map. See https://nodejs.org/api/packages.html#packages_package_entry_points
Is there any specific reason why you don't re-export those files from the entry point (in relay-hooks) and import it from there (in react-relay-offline)?
There was a problem hiding this comment.
Also related to #161 (comment)
I noticed that the esm version was broken due to @restart/hooks not being esm compatible and the es module export entry in the package.json missing.
I removed the @restart/hooks dependency in favor of a simply inline of the logic and also adjusted the package.json for the es export.
With those changes, relay-hooks can be successfully used with snowpack.