Skip to content
New issue

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

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

Already on GitHub? # to your account

Use stat instead of extname when creating require #4860

Merged
merged 1 commit into from
Oct 7, 2020
Merged

Use stat instead of extname when creating require #4860

merged 1 commit into from
Oct 7, 2020

Conversation

adambrgmn
Copy link
Contributor

@adambrgmn adambrgmn commented Oct 5, 2020

This PR will make use of fs.statSync instead of path.extname when creating the custom require function. The issue with the previous version is discussed in #4859. But to summarize the previous version creates issues when the folder the script is running from has a name ending with something that might be mistaken for a file extension – for example mysite.com (path.extname('mysite.com') === '.com').

One could argue if we even need to do the check since makeDefaultLoader is always passed context.cwd which I guess is always a directory? We could probably just do with from = path.join(from, '__fake.js'). Please let me know if you think I should remove the check completely.

Related #4859

@changeset-bot
Copy link

changeset-bot bot commented Oct 5, 2020

🦋 Changeset detected

Latest commit: 0b1a2dd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@graphql-codegen/cli Patch
@graphql-cli/codegen Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@openscript
Copy link

Ran into the same issue and this PR fixed it.

@dotansimha
Copy link
Owner

This is great. Thank you @adambrgmn !

@dotansimha dotansimha merged commit 186962c into dotansimha:master Oct 7, 2020
@adambrgmn adambrgmn deleted the adambrgmn/fix-create-require branch October 7, 2020 15:11
@openscript
Copy link

Would be great to have a release for this soon, so my colleagues don't need to relay on a local build of it.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants