Skip to content

Fix NPM shim resolver on Firefox #290

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

Merged
merged 7 commits into from
Jul 21, 2022
Merged

Fix NPM shim resolver on Firefox #290

merged 7 commits into from
Jul 21, 2022

Conversation

JordanMartinez
Copy link
Contributor

Description of the change

Fixes #289. However, trying to run the React examples from the cookbook get a weird error even with this fix:

2022-07-16_11-34


Checklist:

  • Added the change to the changelog's "Unreleased" section with a reference to this PR (e.g. "- Made a change (#0 by @)")
  • Linked any existing issues or proposals that this pull request should close
  • Updated or added relevant documentation
  • Added a test for the contribution (if applicable)

@JordanMartinez
Copy link
Contributor Author

Looks like the import map for react-dom/server (https://ga.jspm.io/npm:[email protected]/server.browser.js) returns this JS:

import"react";import"object-assign";export{default}from"./cjs/react-dom-server.browser.production.min.js";

I'll investigate.

Production version will use minified JS.
@JordanMartinez
Copy link
Contributor Author

The import map generator allows you to use either a "production" or "development" version of the dependencies. For our import maps, the mode only affects the react libraries. Previously, we were on the 'production' version. The last commit switches to the 'development' version. Since the development version uses non-minified JS code, identifiers like createRoot still exist rather than being minified to something like e.

With the latest commit, the ButtonReactHooks example now works:
2022-07-16_11-53

Comment on lines 28 to 30
"react": "https://ga.jspm.io/npm:[email protected]/dev.index.js",
"react-dom": "https://ga.jspm.io/npm:[email protected]/dev.index.js",
"react-dom/server": "https://ga.jspm.io/npm:[email protected]/dev.server.browser.js",
Copy link
Contributor

@pete-murphy pete-murphy Jul 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder, does bumping the version also fix this? I would think this might need to be in sync with the purescript-react purescript-react-basic libraries which were recently updated to 18 (I'm actually surprised that createRoot is in the dev version of 16.13.1).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you're right. Bumping the React version to 17.0.2, the last release before 18, makes it work.

@JordanMartinez
Copy link
Contributor Author

@thomashoneyman Can I get a review on this when you get a chance?

@JordanMartinez JordanMartinez merged commit ea63671 into purescript:master Jul 21, 2022
@JordanMartinez JordanMartinez deleted the jam/es-module-shims-1.5.9 branch July 21, 2022 12:04
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.

NPM dependency shims no longer work on Firefox 102.1?
3 participants