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

Re-enable eslint import plugin #570

Closed
twschiller opened this issue Jun 21, 2021 · 4 comments · Fixed by #761
Closed

Re-enable eslint import plugin #570

twschiller opened this issue Jun 21, 2021 · 4 comments · Fixed by #761

Comments

@twschiller
Copy link
Contributor

twschiller commented Jun 21, 2021

Context:

"plugin:import/errors",   
"plugin:import/warnings",
@twschiller twschiller changed the title Re-add eslint import plugin Re-enable eslint import plugin Jun 21, 2021
@fregante
Copy link
Collaborator

fregante commented Jul 2, 2021

I can fix this by the way. Eslint does not need the fs: false part in webpack’s config, so I can exclude it from the import:

  • quick fix: just delete the property after import
  • good fix: further clean/deduplicate config

@twschiller
Copy link
Contributor Author

Would be good to fix, especially as we'll want to use to help with modules with broken tree-shaking. I'd err on the side of the quick fix, but not sure I understand the alternatives

exclude it from the import

Which import?

further clean/deduplicate config

I'm not sure I follow - what would this involve?

@fregante
Copy link
Collaborator

fregante commented Jul 2, 2021

exclude it from the import

Which import?

I thought we were requireing the webpack file so I thought we could process it first, but instead we just pass a file path to it:

"config": "./browsers/webpack.config.js"

Maybe the quick fix isn't that quick. Maybe ESLint can be detected from process.env inside webpack.config.js instead?

further clean/deduplicate config

I'm not sure I follow - what would this involve?

I haven't looked into the details, but the aliases are defined in 2/3 files (including Jest’s config) so we should merge those while also exporting a eslint-compatible webpack configuration that solves this issue. How exactly I don't know yet.

@fregante
Copy link
Collaborator

fregante commented Jul 2, 2021

To be clear, this is the problematic object, sorry for being vague:

fallback: {
fs: false,
crypto: false,
console: false,
vm: false,
path: false,
},

ESLint doesn't need this so we can just somehow exclude this specific part from webpack’s config when ESLint imports the file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants