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

Make minified source available to all tools #702

Closed
wants to merge 1 commit into from

Conversation

david-bezero
Copy link

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and couldn’t find anything (or linked relevant results below)
  • If applicable, I’ve added docs and tests (not applicable)

Description of changes

Support other tools which cannot work with ES Modules (e.g. Jest). The necessary code is already available but using a proprietary key in package.json (for unpkg). This adds standards-compliant config to make it available to other tooling which needs it.

Support other tools which cannot work with ES Modules (e.g. Jest)
@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Jul 29, 2022
@david-bezero
Copy link
Author

the test failure seems to be because some build tooling is incompatible with the latest change to (experimental) loaders in Node 18.6 (unrelated to this change)

@ChristianMurphy
Copy link
Member

There is a dual package hazard when exporting both CJS and ESM.
It has been discussed at some length at remarkjs/remark#969 (comment) and unifiedjs/unified#121 (comment)
It can cause real problems, trying to work around Jest-specific limitations.

There are some suggestions on how to make Jest play nicely with ESM in #635

@ChristianMurphy ChristianMurphy added the 🙅 no/wontfix This is not (enough of) an issue for this project label Jul 29, 2022
@github-actions

This comment has been minimized.

@github-actions github-actions bot added 👎 phase/no Post cannot or will not be acted on and removed 🤞 phase/open Post is being triaged manually labels Jul 29, 2022
@david-bezero
Copy link
Author

@ChristianMurphy I guess note that the dual package hazard only applies to packages which contain meaningful global state (which, as far as I can see, this one doesn't). Also you're already providing the dual packages; this PR just makes them both available to the tooling, so I hope you might reconsider this decision.

For now, the only workaround available to users is this rather nasty config:

  moduleNameMapper: {
    'react-markdown': '<rootDir>/node_modules/react-markdown/react-markdown.min.js',
  },

@ChristianMurphy
Copy link
Member

The dual package hazard has several pitfalls and risks, global state is just one of them.
In addition, some remark/rehype plugins do use global state.

Having to configure around a bug/limitation of Jest is an acceptable trade-off, vs the risks and known issues of dual packaging.
And I'm hopeful in the future Jest will make ESM fully supported and seamless jestjs/jest#9430

@remarkjs remarkjs locked as resolved and limited conversation to collaborators Aug 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🙅 no/wontfix This is not (enough of) an issue for this project 👎 phase/no Post cannot or will not be acted on
Development

Successfully merging this pull request may close these issues.

2 participants