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

repro: superstruct@2.0.0 causes errors for repos with moduleResolution set to Node{16,Next} #4458

Closed
wants to merge 6 commits into from

Conversation

MajorLift
Copy link
Contributor

@MajorLift MajorLift commented Jun 24, 2024

Explanation

The changes introduced in superstruct v2.0.0-0 don't enable us to go back to using superstruct.

To demonstrate, I created a repro branch where all dependencies and nested dependencies are forced to use superstruct v2.0.0-0 via yarn resolutions.

In the ci build run results, we still see a large number of the following error for type imports when Node{16,Next} is enabled:

error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'.

https://github.com/MetaMask/core/actions/runs/9654219968/job/26628058963?pr=4458

The reason for this is that our libraries are CJS and superstruct is ESM, and with Node{16,Next} enabled, a CJS project can only import types from an ESM project if it exports type declarations files that are unambiguously specified as .d.cts. Ambiguous .ts and .d.ts files that are interoperable between CJS/ESM are no longer supported in Node{16,Next}.

References

@MajorLift MajorLift force-pushed the repro/superstruct@2.0.0-still-causes-errors branch from 9235083 to 533251c Compare June 24, 2024 23:56
@MajorLift MajorLift force-pushed the repro/superstruct@2.0.0-still-causes-errors branch from 4e9b894 to bff42e7 Compare July 27, 2024 17:59

This comment was marked as resolved.

Copy link

socket-security bot commented Jul 27, 2024

👍 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: npm/@aashutoshrathi/word-wrap@1.2.6, npm/@babel/compat-data@7.24.4, npm/@babel/core@7.24.4, npm/@babel/helper-annotate-as-pure@7.22.5, npm/@babel/helper-compilation-targets@7.23.6, npm/@babel/helper-create-class-features-plugin@7.24.4, npm/@babel/helper-member-expression-to-functions@7.23.0, npm/@babel/helper-optimise-call-expression@7.22.5, npm/@babel/helper-replace-supers@7.24.1, npm/@babel/helper-skip-transparent-expression-wrappers@7.22.5, npm/@babel/helper-validator-option@7.23.5, npm/@babel/helpers@7.24.4, npm/@babel/plugin-syntax-jsx@7.24.1, npm/@babel/plugin-transform-typescript@7.24.4, npm/@babel/preset-typescript@7.24.1, npm/@eslint-community/regexpp@4.10.0, npm/@jridgewell/sourcemap-codec@1.4.15, npm/@lavamoat/aa@4.2.0, npm/@lavamoat/allow-scripts@3.0.4, npm/@metamask/create-release-branch@3.0.0, npm/@noble/curves@1.3.0, npm/@npmcli/fs@3.1.0, npm/@npmcli/git@5.0.5, npm/@npmcli/package-json@5.0.2, npm/@npmcli/promise-spawn@7.0.1, npm/@scure/bip32@1.3.3, npm/@scure/bip39@1.2.2, npm/@types/babel__traverse@7.20.5, npm/@types/lodash@4.17.0, npm/@types/node@16.18.96, npm/@vue/compiler-core@3.4.21, npm/@vue/compiler-dom@3.4.21, npm/@vue/compiler-sfc@3.4.21, npm/@vue/compiler-ssr@3.4.21, npm/@vue/shared@3.4.21, npm/acorn@8.11.3, npm/bin-links@4.0.3, npm/browserslist@4.23.0, npm/cacache@18.0.2, npm/caniuse-lite@1.0.30001608, npm/cjs-module-lexer@1.2.3, npm/cmd-shim@6.0.2, npm/debug@4.3.4, npm/debug@4.3.6, npm/electron-to-chromium@1.4.733, npm/eslint-plugin-promise@6.1.1, npm/esquery@1.5.0, npm/ethereum-cryptography@2.1.3, npm/ethers@6.12.1, npm/foreground-child@3.1.1, npm/globalthis@1.0.3, npm/hosted-git-info@7.0.1, npm/https-proxy-agent@7.0.4, npm/import-local@3.1.0, npm/ini@1.3.8, npm/is-core-module@2.13.1, npm/json-parse-even-better-errors@3.0.1, npm/jsonc-parser@3.2.1, npm/make-fetch-happen@13.0.0, npm/micromatch@4.0.5, npm/minipass-fetch@3.0.4, npm/node-gyp@10.1.0, npm/node-releases@2.0.14, npm/nopt@7.2.0, npm/normalize-package-data@6.0.0, npm/npm-package-arg@11.0.1, npm/npm-pick-manifest@9.0.0, npm/nwsapi@2.2.7, npm/object-inspect@1.13.1, npm/optionator@0.9.3, npm/pony-cause@2.1.10, npm/postcss@8.4.38, npm/prettier-plugin-packagejson@2.5.0, npm/proc-log@3.0.0, npm/socks-proxy-agent@8.0.3, npm/spdx-license-ids@3.0.17, npm/ssri@10.0.5, npm/synckit@0.9.0, npm/tough-cookie@4.1.3, npm/update-browserslist-db@1.0.13, npm/validate-npm-package-name@5.0.0, npm/ws@8.5.0, npm/yaml@2.4.1

View full report↗︎

Next steps

Take a deeper look at the dependency

Take 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 package

If 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 risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/foo@1.0.0 or ignore all packages with @SocketSecurity ignore-all

@MajorLift MajorLift force-pushed the repro/superstruct@2.0.0-still-causes-errors branch from bff42e7 to a1285a7 Compare July 27, 2024 18:03
@MajorLift
Copy link
Contributor Author

@SocketSecurity ignore-all

@MajorLift MajorLift closed this Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant