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

Unable to load @react-aria/textfield as ESM module #2881

Closed
jwnx opened this issue Feb 23, 2022 · 19 comments · Fixed by #3630 or #4038
Closed

Unable to load @react-aria/textfield as ESM module #2881

jwnx opened this issue Feb 23, 2022 · 19 comments · Fixed by #3630 or #4038
Assignees
Labels
enhancement New feature or request

Comments

@jwnx
Copy link

jwnx commented Feb 23, 2022

🐛 Bug Report

Starting with version 3.5.1, the contents of @react-aria/textfield/dist/module.js seems to have become garbled. The file at version 3.5.0 looks good and can be correctly loaded as an ESM module.

🤔 Expected Behavior

The file to be loaded correctly, for example, by jest.

😯 Current Behavior

The module is not understood by node:

SyntaxError: The requested module '@react-aria/textfield' does not provide an export named 'useTextField'

💁 Possible Solution

🔦 Context

Trying to run unit tests against a component that depends on @react-aria/textfield.

💻 Code Sample

🌍 Your Environment

Software Version(s)
react-spectrum 3.5.2
Browser Firefox
Operating System macOS

🧢 Your Company/Team

🕷 Tracking Issue (optional)

@devongovett
Copy link
Member

The last line does have the export defined... so not sure what would be happening.

export {$7811e7c771d6e497$export$712718f7aec83d5 as useTextField, $7f9aed3807b71324$export$4f384c9210e583c3 as useFormattedTextField};

@LFDanLu LFDanLu added the waiting Waiting on Issue Author label Feb 23, 2022
@beamery-tomht
Copy link
Contributor

Similar issue moving from @react-aria/utils": "3.11.0 to @react-aria/utils": "3.11.1 or 3.11.2. It worked fine before, change the version and import { useId } from '@react-aria/utils' doesn't work in ts-node/esm loader for mocha tests.

@beamery-tomht
Copy link
Contributor

Had a quick skim over your history for react-aria/utils to see what changes might have introduced it - only thing that stands out is upgrading parcel

@devongovett
Copy link
Member

Could someone provide reproduction steps for this?

@kherock
Copy link
Contributor

kherock commented May 11, 2022

Possibly related, I'm having issues using these components in NextJS SSR while native ESM is enabled due to a missing "type": "module" or .mjs file extension on the module files. NextJS is resolving the "module" path, but when Node tries to load it, it expects CJS without either of the criteria for ESM being fulfilled.

@kherock
Copy link
Contributor

kherock commented May 11, 2022

Ok I figured it out, this particular problem is an issue with the CJS export. Node.js does attempt to load ESM exported by this library unless explicitly asked to (e.g. import '@react-aria/textfield/dist/module.js').

compare these two and scroll to the bottom:
https://unpkg.com/@react-aria/textfield@3.5.0/dist/main.js
https://unpkg.com/@react-aria/textfield@3.5.1/dist/main.js

Node.js has a somewhat flaky heuristic for providing named exports for CJS: https://nodejs.org/api/esm.html#commonjs-namespaces

The important line is here

Live binding updates or new exports added to module.exports are not detected for these named exports.

Parcel 2 changed from assigning to exports to using binding function with parcel$exportWildcard.

To reproduce, just run a script ending in .mjs in Node.js, with contents like

import { mergeProps } from '@react-aria/utils'; // v3.12.0

Then downgrade utils to 3.11.0 and watch the error go away

@beamery-tomht
Copy link
Contributor

Bump. Any news on this?

@kherock
Copy link
Contributor

kherock commented Aug 9, 2022

@devongovett any way I can help on this? I'm blocked from updating any dependencies until this is resolved.

I see two options:

  1. find a way to have Parcel revert back to assigning to exports normally, removing the parcel$exportWildcard helper
  2. expose an exports map with a proper ESM export that Node can use

I'm not sure if I have the bandwidth to look at option 2. since I think it would involve proposing an implementing a solution to parcel-bundler/parcel#4155. I could probably could come up with a workaround using the release scripts in the meantime.

@devongovett
Copy link
Member

What is causing a CJS module to be loaded as ESM in the first place?

@kherock
Copy link
Contributor

kherock commented Aug 9, 2022

CJS isn't loading as ESM, Node is just statically analyzing it for named exports. This analysis happens when you request a module like the following from an ESM context (e.g. a file ending in .mjs)

import { mergeProps } from '@react-aria/utils';

And the reason Node is even looking at the CJS file in the first place is because its ESM loader will only look for it at .exports.import or .exports.default1

Footnotes

  1. Additionally, the entry at .exports.import needs to end in .mjs, or the whole package otherwise will need "type": "module". Without this, Node will attempt parsing the file as CJS and throw at the first instance of import

@devongovett
Copy link
Member

I got that, but where is this ESM import of a React Aria package running in node? You mentioned Next.js but I thought they used CJS.

@kherock
Copy link
Contributor

kherock commented Aug 9, 2022

NextJS uses ESM by default as of version 12!

@devongovett
Copy link
Member

Hmm, I have a test project using next 12.1.6 and it seems to work fine... https://github.com/devongovett/rsp-next Any other configuration?

@mquandalle
Copy link

I have this issue with viteJS in SSR mode which also use ESM by default in V3
https://vitejs.dev/guide/migration.html#architecture-changes-and-legacy-options

corresponding error in my app

@devongovett
Copy link
Member

You could use a CJS require in the meantime, or transpile your code to CJS. Node ESM has never really been supported by React Aria. It may have worked by accident sometimes due to node's flaky interop, but we don't currently test against it.

If we wanted to support it, I guess using an exports map would be the right way, though I suspect some other things might be broken as well. Node's ESM is quite strict compared with bundlers/transpilers - for example, relative imports must use file extensions.

@vstreame
Copy link

We're blocked on this as well. Is there a workaround anyone as found? Currently if I rewrote the output to use normal module.exports.foobar = value syntax rather than $parcel$export it solves the issue for NodeJS. But I would like to avoid needing to patch-package everything.

@snowystinger snowystinger mentioned this issue Oct 12, 2022
5 tasks
@dannify dannify moved this from 📋 Waiting for Sprint to 🏗 In Progress in RSP Component Milestones Oct 12, 2022
@snowystinger snowystinger moved this from 🏗 In Progress to 👀 In Review in RSP Component Milestones Oct 13, 2022
Repository owner moved this from 👀 In Review to ✅ Done in RSP Component Milestones Nov 22, 2022
@snowystinger
Copy link
Member

I've merged this branch, we're going to have it in nightlies for a while to make sure we haven't broken anything. Our targeted release date for this is January. Please try it out on the nightlies which publish at 2am PDT.
"0 9 * * *" # 02:00 PDT
First one will be tonight.
Thank you for helping us with this work!

@devongovett devongovett reopened this Dec 12, 2022
Repository owner moved this from ✅ Done to ✏️ To Groom in RSP Component Milestones Dec 12, 2022
@devongovett
Copy link
Member

Unfortunately, we had to revert this for now. It broke webpack 4, which still has more downloads per week than webpack 5. See #3839 for details. We may be able to regroup in the new year, but so far we haven't found a solution that works across all tools.

@matthewdeutsch matthewdeutsch moved this from ✏️ To Groom to 📋 Waiting for Sprint in RSP Component Milestones Jan 4, 2023
@snowystinger snowystinger moved this from 📋 Waiting for Sprint to 🏗 In Progress in RSP Component Milestones Jan 21, 2023
@devongovett
Copy link
Member

Update: I think I got a new approach working in #4038. We will do some more testing to be sure.

@devongovett devongovett moved this from 🏗 In Progress to 👀 In Review in RSP Component Milestones Feb 14, 2023
@github-project-automation github-project-automation bot moved this from 👀 In Review to ✅ Done in RSP Component Milestones Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

8 participants