Build package as both CJS and ESM

[Mrtenz]

Instead of transpiling the source code to just CJS, we now transpile it to ESM as well. This is beneficial for projects using bundlers that are capable of tree-shaking (such as MetaMask Snaps), because it's much easier to determine whether the code has side effects or not.

For the transpilation, we now use SWC rather than TSC. This avoids having to do type checking twice, which is quite slow. TypeScript is still used for generating the declaration files, however, so the code is still being type-checked.

This is based on a change in the Snaps repository (MetaMask/snaps#1519), which has been tested in the extension, so this shouldn't cause any problems there.

[socket-security]

👍 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: [email protected], [email protected], [email protected], @mole-inc/[email protected], @swc/[email protected], @swc/[email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], @tokenizer/[email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], @szmarczak/[email protected], @types/[email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]

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 package-name@version specifiers. e.g. @SocketSecurity ignore [email protected] bar@* or ignore all packages with @SocketSecurity ignore-all

[socket-security]

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages	Version	New capabilities	Transitives	Size	Publisher
@swc/cli	0.1.62	network, filesystem, shell, environment	+83	386 MB	kdy1
@swc/core	1.3.68	filesystem, shell, environment	+10	384 MB	kdy1

[FrederikBolding]

We probably don't need to run build on all these Node versions. But can be fixed in another PR.

[Mrtenz]

Agreed, but out of scope for this PR.

[mcmire]

Very cool! Just had a few comments/suggestions.

[mcmire]

Looks good!

[Mrtenz]

After some more testing, I found that Browserify does not support the exports field (browserify/resolve#222). This means that we can't use any scoped imports, as they will not work in the extension. For example, if @metamask/foo contains the following import:

import { bar } from '@metamask/bar/baz'; // dist/cjs/baz/index.js

This will work fine in Node.js, Webpack, etc., but in Browserify this results in an error:

Could not resolve "@metamask/bar/baz"

I have an idea on how this can be solved, but will put it into draft until this has been tested.

[Gudahtt]

Could you elaborate on the connection between that problem, and the contents of this PR? I see that this PR adds the exports field, but it's less clear to me how this impacts browserify (i.e. does it matter that browserify can still only use the main/module entrypoint?)

[Mrtenz]

Could you elaborate on the connection between that problem, and the contents of this PR? I see that this PR adds the exports field, but it's less clear to me how this impacts browserify (i.e. does it matter that browserify can still only use the main/module entrypoint?)

Sure. The problem is that packages that don't use Browserify could import in ways that are not supported by Browserify. For example, if @metamask/utils imports from @metamask/key-tree/some/path, and @metamask/utils gets imported into the extension, Browserify would fail because node_modules/@metamask/key-tree/some/path does not exist. It doesn't look at the exports field to try and resolve the proper path, i.e., node_modules/@metamask/key-tree/dist/cjs/some/path.

We solved this in the Snaps repo by explicitly exporting ./dist/cjs/* and ./dist/esm/*, rather than ./*. This means that @metamask/utils can import @metamask/key-tree/dist/cjs/some/path, because it's specified in the exports field, and Browserify can import it, because it's an actual path on the disk: node_modules/@metamask/key-tree/dist/cjs/some/path exists.

Ideally we avoid importing from package/dist in the first place, but since we're doing that, I think this is the best solution as long as we're still using Browserify.

I hope that makes sense!

[Gudahtt]

Thanks! Makes sense.

Perhaps in the meantime we could use the main and module fields to support multiple build types? Reserving the nicer export entry points for later, when that problem is resolved.

[Mrtenz]

Thanks! Makes sense.

Perhaps in the meantime we could use the main and module fields to support multiple build types? Reserving the nicer export entry points for later, when that problem is resolved.

I hadn't even considered that we don't need exports at all in that case. 😅

[mcmire]

LGTM!

[Gudahtt]

LGTM!

[Gudahtt]

@SocketSecurity ignore @swc/[email protected]
@SocketSecurity ignore @swc/[email protected]

These packages are expected to have filesystem and shell access. And core needs a postinstall script to prepare native code.

[Gudahtt]

@SocketSecurity ignore-all

The remaining alerts are regarding packages used by SWC with access that seems reasonable for them to have, and new author/unmaintained warnings (none of which seem suspicious).

[Mrtenz]

Just leaving a comment here for future reference.

When we change the package to use SWC for other parts of the stack, such as testing, we need to keep in mind to use a different .swcrc file. This one is explicitly ignoring test files and such, so will not work with Jest.