Skip to content
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

we have trouble importing import splideStyle from '@splidejs/react-splide/css/core'; #6187

Closed
balazsbajorics opened this issue Aug 6, 2024 · 1 comment · Fixed by #6425
Closed
Assignees
Labels
Bug Something isn't working

Comments

@balazsbajorics
Copy link
Contributor

the package json has these dependencies:

"@splidejs/react-splide": "^0.7.12",
"@splidejs/splide-extension-auto-scroll": "^0.5.3",

put the line

import splideStyle from '@splidejs/react-splide/css/core'; anywhere in a remix route, observe the error message:

Image

@balazsbajorics balazsbajorics added the Bug Something isn't working label Aug 6, 2024
@gbalint gbalint self-assigned this Aug 9, 2024
@gbalint
Copy link
Contributor

gbalint commented Sep 16, 2024

@Splidejs has a module format which we don't support yet.

This is the problematic import: import splideStyle from '@splidejs/react-splide/css/core'

In the module resolution algorithm the module specifier is the string before the 2nd "/", so it is @splidejs/react-splide. And there is actually a package.json there, which has an exports field, which resolves the path after the module specifier (/css/core) to a file (./dist/css/splide-core.min.css)
See LOAD_PACKAGE_EXPORTS(X, DIR) in https://nodejs.org/api/modules.html#all-together

Our resolution algorithm fails to find this for multiple reasons:

  • We treat the whole toImport string as the module specifier, not just the part before the 2nd slash.
  • We don't parse the exports field from package.json
  • We don't use the data in the exports field to resolve the returned path.

I started to write a very rough draft spike to do these, I managed to find and parse the package.json, and return the path from the exports field in package.json. It doesn't work yet, but maybe it is helpful: #6224

Let me note, that I don't think it is a good idea to quick fix the current solution, I would make sure we implement the full module resolution algorithm described in the commonjs docs.

@Rheeseyb Rheeseyb self-assigned this Sep 16, 2024
liady pushed a commit that referenced this issue 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants