-
Notifications
You must be signed in to change notification settings - Fork 346
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
chore: migrate affected tests to esm #4158
Conversation
@@ -73,7 +73,7 @@ | |||
"postinstall": "node ./scripts/postinstall.js" | |||
}, | |||
"config": { | |||
"eslint": "--ignore-path .gitignore --cache --format=codeframe --max-warnings=0 \"{src,tools,scripts,site,tests,.github}/**/*.{mjs,cjs,js,md,html}\" \"*.{mjs,cjs,js,md,html}\" \".*.{mjs,cjs,js,md,html}\"", | |||
"eslint": "--ignore-path .gitignore --cache --format=codeframe --max-warnings=0 \"{src,scripts,site,tests,.github}/**/*.{mjs,cjs,js,md,html}\" \"*.{mjs,cjs,js,md,html}\" \".*.{mjs,cjs,js,md,html}\"", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed to exclude tools
from eslint as it hangs up on one file:
npx eslint ./tools/project-graph/file-visitor.mjs
does not work cc @ehmicky maybe you have a clou for me it seems that some lint rules collide here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be a problem with eslint-plugin-import
running very slowly when the file being linted has an import ts from 'typescript'
statement. This bug report might be the same problem: import-js/eslint-plugin-import#2340
The following seems to fix the problem: changing:
import ts from 'typescript'
To:
const { default: ts } = await import(`${'typescript'}`)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ehmicky yea you are right it passed locally after 13min 🤯 nevertheless I think we should not write our code different because a lint rule is not playing well. I would rather opt then for disabling this lint rule. IMO
📊 Benchmark resultsComparing with 3dd9f74 Package size: 362 MB⬇️ 0.00% decrease vs. 3dd9f74
Legend
|
aa12c1e
to
6cf8b6b
Compare
🎉 Thanks for submitting a pull request! 🎉
Summary
part of the partial esm migration
For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)