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

fix require path to resource in node_modules #7146

Merged

Conversation

birkskyum
Copy link
Contributor

@birkskyum birkskyum commented Sep 10, 2024

Fix #7145

Instead of providing the relative path to the node_modules, let the runtime resolve it.

It's a bit speculative, but it's easy to imagine setups (different package managers, monorepos) that doesn't have the maplibre-gl node module located exactly in the location specified:

  • pnpm e.g. nests below a node_modules/.pnpm/maplibre-gl
  • monorepos often have node_modules at root but not in each package

@archmoj I don't have a repro of the error, and thus haven't been able to validate the patch. Also, I'm trying to wrap my head around why it's at all necessary for the users of a plotly.js distribution to resolve this path, because I expected webpack to have done so already when the plotly.js bundles are created.

Update: it's not using a pre-build dist, because of this

"main": "./lib/index.js",

@birkskyum
Copy link
Contributor Author

@archmoj , I have found in a reproduction that:

  • Works already: `import Plotly from "plotly.js/dist/plotly.min.js"
  • Broken, but fixed in this PR: `import Plotly from "plotly.js/dist/plotly.js"
  • Broken still after this PR: `import Plotly from "plotly.js"

@archmoj
Copy link
Contributor

archmoj commented Sep 10, 2024

Nicely done.
Thanks very much for the fix.
💃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't resolve '../node_modules/maplibre-gl/dist/maplibre-gl.css'
3 participants