-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
Convert package to ESM #113
Conversation
00abc15
to
717f9a3
Compare
New, updated, and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: [email protected] |
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring: Next stepsTake a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with |
366a95b
to
909aef0
Compare
@SocketSecurity ignore @inquirer/[email protected] [email protected] [email protected] [email protected] I've checked all of these, the authors seem legit. |
@SocketSecurity ignore [email protected] |
@rekmarks Sorry for the conflicts. We just merged a PR that had a wide array of changes. I want to get this PR in the next release though, so I'm happy to work on resolving these conflicts for you — just let me know. |
@mcmire no, no: someone had to suffer here, and under the circumstances it should be me. I'll resolve the conflicts! |
Alright, this is ready for re-review! |
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.
Looks good!
This PR updates `execa` to `^8.0.1`. Since `execa@>=6.0.0` is ESM-only and `jest` only has experimental ESM support (jestjs/jest#10976), this required switching from `ts-jest` to `babel-jest`. To minimize dependency transpilation, the ESM packages that are necessary to transpile are enumerated in `jest.config.js`. This version of `execa` includes [automatic escaping of shell arguments](https://github.com/sindresorhus/execa/tree/v8.0.1#execafile-arguments-options), which was the entire point of #112, #113, and this PR. The state of ESM support in the Node.js ecosystem is absolutely horrible, and I would not recommend further migrations for the time being. We should continue to dual-release our packages and avoid ESM-only dependencies until the ecosystem has matured. For details see the above `jest` issue and nodejs/node#37648.
Converts this package to an ESM module. Kudos to this guide and this issue: TypeStrong/ts-node#1997
In our organization, this is only possible for this package because it is not used anywhere else, as we rely on CommonJS extensively.
Changes in detail:
package.json
for ESM compatibilityrequire()
withimport
statements.js
file extension to all file imports in TypeScript source filestsconfig.json
)module
/moduleResolution
fields toNode16
.Node16
overNodeNext
, as the behavior ofNodeNext
may change in the future. See here for details..eslintrc.cjs
)parseOptions.sourceType = 'module'
jest.config.cjs
)moduleNameMapper
to strip the.js
from local file imports. See here for details.ts-node
withtsx
, which has better ESM compatibility.jest-it-up
--config
option (thank you @PeterYinusa! feat(config): add -c, --config option rbardini/jest-it-up#12) to target our new.cjs
config file.