-
Notifications
You must be signed in to change notification settings - Fork 570
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
Build packages with tsup
#2120
Build packages with tsup
#2120
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 |
just a general comment on it can make debugging a huge pain. we've since moved away from |
Thanks for the heads up! I'll see how big of a difference |
@SocketSecurity ignore npm/tree-kill@1.2.2 |
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.
tsup
moves everything to separate chunk files (when code splitting is enabled), which breaks code relying on relative imports, in our case, the Webpack loader. As a workaround I added this new loader, which executes a function. This works since the worker path points to __filename
, so regardless of where the file is placed, it will load properly.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## tsup #2120 +/- ##
=======================================
Coverage 96.59% 96.60%
=======================================
Files 332 334 +2
Lines 7582 7595 +13
Branches 1175 1175
=======================================
+ Hits 7324 7337 +13
Misses 258 258 ☔ View full report in Codecov by Sentry. |
@SocketSecurity ignore npm/mz@2.7.0 |
@@ -1,35 +1,14 @@ | |||
{ | |||
"name": "@metamask/snaps-simulator", | |||
"version": "2.4.3", | |||
"private": true, |
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.
Note that I added private: true
here. We previously published this to NPM for snaps-jest
, but we no longer use it.
@@ -100,38 +73,6 @@ jobs: | |||
exit 1 | |||
fi | |||
|
|||
post-build: |
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.
I've removed all post-tsc scripts for simplicity.
conditions: os=freebsd & cpu=x64 | ||
languageName: node | ||
linkType: hard | ||
|
||
"@esbuild/linux-arm64@npm:0.18.20": |
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.
Can we dedupe our esbuild deps or does that require bumping WDIO again?
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 like it requires bumping WDIO yeah.
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.
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.
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.
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: - All packages with Node.js-specific code have been refactored to make them browser compatible. Node.js exports were moved to a separate `/node` export, i.e., `@metamask/snaps-utils/node`. - This also applies to the React Native webview service in `snaps-controllers`, which can now be imported from `@metamask/snaps-controllers/react-native`. - The `snaps-simulator` package is no longer published to NPM. We were previously using it for `snaps-jest`, but it's no longer used now. --- This pull request consists of the following pull requests: - #2120. - #2211.
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.
This changes all packages to be built with
tsup
, instead of SWC.tsup
usesesbuild
under the hood, so performance should be comparable.More context here: MetaMask/utils#144.