-
Notifications
You must be signed in to change notification settings - Fork 172
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
Use node module resolution #6425
Merged
Merged
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
#14708 Bundle Size — 57.94MiB (+0.01%).ab16941(current) vs 916e8b9 master#14701(baseline) Warning Bundle contains 70 duplicate packages – View duplicate packages Bundle metrics
|
Current #14708 |
Baseline #14701 |
|
---|---|---|
Initial JS | 40.95MiB (+0.02% ) |
40.94MiB |
Initial CSS | 0B |
0B |
Cache Invalidation | 18.46% |
17.91% |
Chunks | 21 |
21 |
Assets | 23 |
23 |
Modules | 4147 (+0.05% ) |
4145 |
Duplicate Modules | 213 |
213 |
Duplicate Code | 27.32% |
27.32% |
Packages | 477 |
477 |
Duplicate Packages | 70 |
70 |
Bundle size by type 2 changes
1 regression
1 improvement
Current #14708 |
Baseline #14701 |
|
---|---|---|
JS | 57.94MiB (+0.01% ) |
57.93MiB |
HTML | 7.37KiB (-0.25% ) |
7.39KiB |
Bundle analysis report Branch fix/node-module-resolution Project dashboard
Generated by RelativeCI Documentation Report issue
Rheeseyb
requested review from
gbalint,
seanparsons,
ruggi,
balazsbajorics,
liady and
bkrmendy
October 1, 2024 11:19
seanparsons
reviewed
Oct 1, 2024
editor/src/core/es-modules/package-manager/module-resolution.ts
Outdated
Show resolved
Hide resolved
seanparsons
approved these changes
Oct 1, 2024
gbalint
reviewed
Oct 2, 2024
editor/src/core/es-modules/package-manager/module-resolution.ts
Outdated
Show resolved
Hide resolved
liady
approved these changes
Oct 8, 2024
balazsbajorics
approved these changes
Oct 8, 2024
balazsbajorics
added a commit
that referenced
this pull request
Oct 11, 2024
This reverts commit da6bf80.
balazsbajorics
added a commit
that referenced
this pull request
Oct 11, 2024
This reverts commit da6bf80. **Problem:** When loading the Hydrogen demo store, an infinite recursion happens: ![image](https://github.com/user-attachments/assets/ee1a761d-c950-43c3-a633-f62ec580d722)
liady
pushed a commit
that referenced
this pull request
Dec 13, 2024
**Problem** Utopia's module resolution logic is based on the [2019 node module resolution](https://web.archive.org/web/20190213102857/https://nodejs.org/api/modules.html#modules_all_together), but that logic has significantly evolved since then. This means that there are plenty of cases where the module resolution whilst running a project in Utopia would not match that whilst running elsewhere, causing certain projects to break in unexpected ways. **Fix:** I've implemented the latest node.js module resolution logic from https://nodejs.org/api/modules.html#all-together. Since this is of course running in the browser, this also still needs to take into account the [`browser` field of the `package.json`](https://github.com/defunctzombie/package-browser-field-spec), which was already implemented but I wanted to call it out as that is one area where this differs from the node.js spec. There is still a remaining `FIXME` in here about partial path matching when checking the `imports` and `exports` fields. I'll tackle that in a followup PR. Fixes #6187
liady
pushed a commit
that referenced
this pull request
Dec 13, 2024
This reverts commit da6bf80. **Problem:** When loading the Hydrogen demo store, an infinite recursion happens: ![image](https://github.com/user-attachments/assets/ee1a761d-c950-43c3-a633-f62ec580d722)
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.
Problem
Utopia's module resolution logic is based on the 2019 node module resolution, but that logic has significantly evolved since then. This means that there are plenty of cases where the module resolution whilst running a project in Utopia would not match that whilst running elsewhere, causing certain projects to break in unexpected ways.
Fix:
I've implemented the latest node.js module resolution logic from https://nodejs.org/api/modules.html#all-together. Since this is of course running in the browser, this also still needs to take into account the
browser
field of thepackage.json
, which was already implemented but I wanted to call it out as that is one area where this differs from the node.js spec.There is still a remaining
FIXME
in here about partial path matching when checking theimports
andexports
fields. I'll tackle that in a followup PR.Fixes #6187