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

Forbid using ! in import paths #733

Closed
gaearon opened this issue Sep 24, 2016 · 8 comments · Fixed by import-js/eslint-plugin-import#586
Closed

Forbid using ! in import paths #733

gaearon opened this issue Sep 24, 2016 · 8 comments · Fixed by import-js/eslint-plugin-import#586

Comments

@gaearon
Copy link
Contributor

gaearon commented Sep 24, 2016

I've heard a few reports that people use Webpack import extension syntax (like !!file!./stuff) to customize Create React App loaders. We don't support this officially, and we may accidentally (or on purpose) break this in the future. This also couples apps to Webpack resolution mechanism too hard.

We need to write a Webpack plugin that throws in parsing stage (https://github.com/webpack/docs/wiki/plugins#the-parser-instance-compilerparser) if you attempt to require or import something with ! in the path.

@jquense
Copy link
Contributor

jquense commented Sep 24, 2016

I'm not certain but I'm pretty sure you can't do that because many loaders, use this syntax internally, as part of their implementation. Not sure you could distinguish between those requests and the user made ones, maybe if you plugin before the loaders run? You may also not be using loaders that do this, but some of "core" ones used to at least

@gaearon
Copy link
Contributor Author

gaearon commented Sep 24, 2016

Yeah, we'd need to limit this error to the code in user app folder.

@jquense
Copy link
Contributor

jquense commented Sep 24, 2016

Yeah, we'd need to limit this error to the code in user app folder.

even still, the loaders will actively change that code, if the plugin runs after the code has been through the loaders, it doesn't matter where it's original location on disk is. Tho I think if you are just throwing an error you probably can run the plugin before any loader activity.

@SpaceK33z
Copy link
Contributor

SpaceK33z commented Sep 24, 2016

Maybe it would be an idea to write / use an ESLint plugin for this then? eslint-plugin-import would be a good place for that.

@fson
Copy link
Contributor

fson commented Sep 25, 2016

Good idea, @SpaceK33z. I think eslint-plugin-import would be an easy and logical place to add this, and this way it also applies to user code only.

I opened a PR in eslint-plugin-import that implements this: import-js/eslint-plugin-import#586

@SpaceK33z
Copy link
Contributor

Nice work @fson!

@fson
Copy link
Contributor

fson commented Sep 30, 2016

eslint-plugin-import 2.0.0 was released yesterday with this new rule. I'm making a pull request to add it in our config today.

@gaearon
Copy link
Contributor Author

gaearon commented Sep 30, 2016

Awesome! Note you'll need to update every place where we hardcoded the versions in docs.
😞

@gaearon gaearon modified the milestones: 0.7.0, 1.0.0 Sep 30, 2016
@fson fson modified the milestones: 0.8.0, 0.7.0 Oct 12, 2016
@fson fson modified the milestones: 0.7.0, 0.8.0 Oct 22, 2016
@fson fson closed this as completed in #803 Oct 22, 2016
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants