-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 support for .cjs .mjs and .js with package.json type="module" #4873
add support for .cjs .mjs and .js with package.json type="module" #4873
Conversation
Hi @jantimon! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
I believe the Relay config must also be able to be read by the Babel Transform which would presumably need an equivalent change? This is further complicated by the fact that there are alternative transform implementations which may also try to read the config. If they do (maybe you could check?) we would probably need to either also update them or at least update their (and our) documentation to callout the incompatability. |
Thanks for your feedback - I've took a quick look into the Babel Plugin and it looks like it is not possible to add it to babel You probably know that Babel and its plugins must be CommonJS and synchronous by design - so we can't use However Next.js (which uses @swc/plugin-relay) is able to consume both relay config formats CJS and ESM: import relayConfig from './relay.config.mjs';
export default {
compiler: {
relay: relayConfig
}
}; I believe your idea to update the babel documentation is the best option |
I added a note for |
This makes sense to me, and the large number of upvotes indicates others feel similarly. One question: how does Babel fail if you use |
@captbaritone has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
I created a minimal webpack / relay / babel project and got a understandable error message:
For details please take a look at the full outputs: Full outputs
|
I added also support for Now users can follow the proposed solution of the error and rename their config to |
@captbaritone merged this pull request in 08e45d7. |
When using Node.js projects with
"type": "module"
in package.json, the relay compiler currently fails to load configuration files due to incompatibilities between ESM and CommonJSSee #4060 (comment)
This pr handle all three module format scenarios:
It works by switching from Node
require()
to dynamicimport()
and using the--input-type=module
flagI also added a new test and verified that this approach works for Node 18.0.0
fixes #4060