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

fix: 🐛#1819 ESM output imports, add file extensions #2344

Closed
wants to merge 2 commits into from
Closed

fix: 🐛#1819 ESM output imports, add file extensions #2344

wants to merge 2 commits into from

Conversation

Banou26
Copy link

@Banou26 Banou26 commented Jan 13, 2020

No description provided.

@Banou26
Copy link
Author

Banou26 commented Jan 14, 2020

Any ideas on why only the Node v12 failed tests ?

Seems like the prettier threw, and after that the ci tests threw.

If we direct me towards the problem i could try to fix it, or if it's just a CI problem and it's not important, you can just review and merge it.

@IvanGoncharov
Copy link
Member

Any ideas on why only the Node v12 failed tests ?

Please run "npm test" or yarn test" locally and fix all errors.
Also can you please explain what you are trying to do and what is your use case for these changes?

@Banou26
Copy link
Author

Banou26 commented Jan 14, 2020

There were no errors during the tests, and for good reasons, this PR doesn't modify the behavior of graphql.

Its only purpose is to make the ESM output of the build ESM compliant by having import and export paths use file extensions, using extension-less import paths result in errors such as this one when using graphql with node in module mode("type"="module").

Error: Cannot find module C:\dev\oz\packages\api\node_modules\graphql\language\source imported from C:\dev\oz\packages\api\node_modules\graphql\language\index.mjs

I briefly described the change this PR would result in, in #1819 (comment)

@IvanGoncharov
Copy link
Member

@Banou26 Thanks for PR 👍
I decided to implement it slight different in #2379
It's now merged and will be released as 15.0.0-rc.2

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 this pull request may close these issues.

2 participants