feat: Add new export for initializeInpageProvider
#391
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as resolved.
This comment was marked as resolved.
Gudahtt
force-pushed
the
add-cjs-subpath-export
branch
2 times, most recently
from
74a85e4 to
8344e64
Compare
Gudahtt
force-pushed
the
add-cjs-subpath-export
branch
2 times, most recently
from
e8d09c2 to
3e58d90
Compare
Add a subpath export for the `initializeInpageProvider.cjs` file. This allows projects with legacy build systems to import this file directly, which can be useful for reducing bundle size (especially for build systems without tree shaking). The `metamask-extension` project uses two build systems: one which obeys export maps, one which ignores them (Webpack and browserify respectively). The existing `initializeInpageProvider` export map entry doesn't work for browserify because it's looking for `initializeInpageProvider.js`. The file extension is wrong.
Add a new top-level export map entry for `initializeProvider` that maps correctly onto CJS and ESM builds, and add a fallback JavaScript file at the root of the repository for build systems that don't support export maps. This gives the best of both worlds: build systems that support exports will behave correctly, and those that don't will get the CJS fallback.
Gudahtt
force-pushed
the
add-cjs-subpath-export
branch
from
3e58d90 to
8d85f6b
Compare
Gudahtt
commented
Nov 27, 2024
| @@ -49,6 +49,16 @@ | |||
| "default": "./dist/initializeInpageProvider.cjs" | |||
| } | |||
| }, | |||
| "./initializeInpageProvider": { | |||
There was a problem hiding this comment.
We can remove the other export entries that are no longer needed in a later PR. That would be a breaking change.
Tracked here: #393
Gudahtt
changed the title
initializeInpageProvider.cjsinitializeInpageProvider
Mrtenz
approved these changes
Nov 27, 2024
7 tasks
github-merge-queue bot
pushed a commit
to MetaMask/metamask-extension
that referenced
this pull request
Nov 27, 2024
## **Description** The `@metamask/providers` package has been updated from v14 to v18. The breaking changes do not require any changes to the extension. They include: * Removal of certain provider features, which were later restored * Updates to JSON-RPC packages that used to impact error serialization in a breaking way, but no longer do (as of rpc-errors v7.0.1) * `webextension-polyfill` added as a peer dependency * Sub-path exports no longer work due to addition of exports and ESM build. Altogether this should result in no functional changes, aside from eliminating some redundant older copies of libraries (reducing bundle size). This update was difficult because of the need for a subpath export in `inpage.js`. We want to import just the `initializeInpageProvider` module without the rest of the package, which means we need to use a subpath export (because browserify doesn't support tree shaking). But this was challenging because the export maps added in v17 prevent the use of any subpath exports not defined in the export map. There was no entry that worked correctly across webpack and browserify. This was resolved by a new entry added in MetaMask/providers#391 (as part of v18.2.0) Changelog: https://github.com/MetaMask/providers/blob/main/CHANGELOG.md#1820 [](https://codespaces.new/MetaMask/metamask-extension/pull/28757?quickstart=1) ## **Related issues** N/A ## **Manual testing steps** N/A ## **Screenshots/Recordings** N/A ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
](https://codespaces.new/MetaMask/metamask-extension/pull/28757?quickstart=1){kind=link}
Magkoooh
approved these changes
Nov 27, 2024
Magkoooh
approved these changes
Nov 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a new top-level export map entry for
initializeInpageProviderthat maps correctly onto CJS and ESM builds, and add a fallback JavaScript file at the root of the repository for build systems that don't support export maps.This gives the best of both worlds: build systems that support exports will behave correctly, and those that don't will get the CJS fallback.
The
metamask-extensionproject uses two build systems: one which obeys export maps, one which ignores them (Webpack and browserify respectively). The existinginitializeInpageProviderexport map entry doesn't work for browserify because it's looking forinitializeInpageProvider.js. The file extension is wrong. This fixes that problem.This strategy was copied from https://github.com/MetaMask/snaps/blob/cbe55deb75e640c03add1ebf9235d4abd513e9d5/packages/snaps-sdk/jsx.js