-
Notifications
You must be signed in to change notification settings - Fork 562
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
BREAKING: Use tsup
and refactor exports
#2210
Conversation
👍 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 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2210 +/- ##
=======================================
Coverage ? 96.60%
=======================================
Files ? 337
Lines ? 7599
Branches ? 1175
=======================================
Hits ? 7341
Misses ? 258
Partials ? 0 ☔ View full report in Codecov by Sentry. |
tsup
and refactor exportstsup
and refactor exports
This changes all packages to be built with `tsup`, instead of SWC. `tsup` uses `esbuild` under the hood, so performance should be comparable. More context here: MetaMask/utils#144.
…js (#2211) This refactors all packages to be browser-first: All APIs and packages used should be compatible with browsers. This means no use of Node.js builtins, or native packages. Node-specific APIs have been moved to a separate export `/node` (e.g., `@metamask/snaps-controllers/node`). For the `snaps-controllers` package I've also added a `/react-native` export, which exports the React Native webview service. This also means we can remove the "browser" field from the packages that were using it. ## Breaking changes - Anything that uses Node.js was removed from the default export, and needs to be imported from `/node`. - The React Native webview service was moved, and needs to be imported from `/react-native`.
@SocketSecurity ignore npm/[email protected] |
// eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires | ||
const { default: baseConfig } = require('../../tsup.config'); | ||
|
||
delete baseConfig.entry; |
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.
Why do we need this?
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.
deepmerge
merges arrays by default, so then tsup
would build all TypeScript files. Since we disable splitting here, we only want index.ts
to be built.
This swaps out the build system for
tsup
, and adds a proper ESM build to each package. In addition to that, the following changes have been made:/node
export, i.e.,@metamask/snaps-utils/node
.snaps-controllers
, which can now be imported from@metamask/snaps-controllers/react-native
.snaps-simulator
package is no longer published to NPM. We were previously using it forsnaps-jest
, but it's no longer used now.This pull request consists of the following pull requests:
tsup
#2120.