Use internal node globals impl #93
Conversation
mraerino
added
the
type: feature
|
Should we put this in a different repository? |
|
@ehmicky feel free to put this in its own repo. i don't have the bandwidth right now. |
| @@ -6,7 +6,7 @@ | |||
| "scripts": { | |||
| "test": "npm run format && npm run test:dev", | |||
| "format": "run-s format:*", | |||
| "format:lint": "eslint --ignore-path .gitignore --fix --cache --format=codeframe --max-warnings=0 \"src/**/*.js\"", | |||
| "format:lint": "eslint --ignore-path .eslintignore --fix --cache --format=codeframe --max-warnings=0 \"src/**/*.js\"", | |||
There was a problem hiding this comment.
It seems that the reason to change this would be not to lint the new files in src/node-compat/assets, is this correct?
If so, I think fixing the linting errors in those files should be the fix instead.
There was a problem hiding this comment.
those files are going to be injected into the user's bundle.
the lint errors are about the parser that eslint uses, which does not like es modules. so no chance to fix it.
see https://github.com/netlify/netlify-plugin-edge-handlers/runs/1210596968#step:5:32
There was a problem hiding this comment.
We should fix .eslintrc.json instead to allow linting both ES modules and CommonJS, e.g. using overrides.
|
To be more precise about what I meant: my intention is to decrease our need to maintain this, not increase it. By using a fork, we could benefit in the future if the upstream issue got eventually fixed. Also, we would benefit from the unit tests. Finally, we could get contributions from the other users who seemed to experience the upstream issue as well. We will still need to maintain this code, regardless of whether the logic is in this repository or another. |
Which problem is this pull request solving?
I ran into issues with the globals plugin being present.
The plugin is unmaintained and it would need this PR to work properly on es2020 code: calvinmetcalf/rollup-plugin-node-globals#25
List other issues or pull requests related to this problem
Addition to #88
Describe the solution you've chosen
I decided to roll our own implementation based on the rollup inject plugin.
This is a very small implementation which i don't expect to change a lot in the future.
Describe alternatives you've considered
We could publish & maintain our own plugin, but this seemed like the easier solution. We can always extract it.
Checklist