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.

Sign up for GitHub

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

Jump to bottom

Kill ESM exports for now, since they break Webpack users #2341

Closed
wants to merge 1 commit into from
Closed

Kill ESM exports for now, since they break Webpack users #2341

wants to merge 1 commit into from

Conversation

dead-claudia
Copy link
Member

@dead-claudia dead-claudia commented Dec 17, 2018 •
edited
Loading

Description

Motivation and Context

It unbreaks the experience for Webpack users. This is a stopgap solution until the Webpack issue linked above is fixed.

How Has This Been Tested?

N/A. It's all package-related.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation change

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated docs/change-log.md

@dead-claudia
Kill ESM exports for now, since they break Webpack users
6ce0368 
- See: MithrilJS#2336
- See: webpack/webpack#8491
- Note: revert this once the above issue is resolved
@dead-claudia dead-claudia added the Type: Bug For bugs and any other unexpected breakage label Dec 17, 2018
@dead-claudia dead-claudia added this to the 2.0.0 milestone Dec 17, 2018
@dead-claudia dead-claudia self-assigned this Dec 17, 2018
@dead-claudia dead-claudia requested a review from a team December 17, 2018 07:41
@dead-claudia
Copy link
Member Author

Note: you can still import mithril/mithril.mjs normally with the workarounds documented in #2336, and you can add the relevant aliases for Rollup as necessary. In case you're curious what modules would need aliased with this patch:

  • mithril → mithril/mithril.mjs
  • mithril/stream → mithril/stream/stream.mjs
  • ospec → ospec/ospec.mjs

This exists purely to unbreak existing Webpack workflows until a fix for #2336/webpack/webpack#8491 lands.

@spacejack
Copy link
Contributor

What about this solution?

@barneycarroll
Copy link
Member

I'm not keen. Mithril's relative insignificance in the face of massively popular OSS projects shouldn't entail we bends over backwards to avoid hitting their bugs, especially when user-land solutions exist.

@dead-claudia
Copy link
Member Author

@barneycarroll Yeah, this was a just in case, and I wasn't going to merge it without discussion.

FWIW, I was only positing it to see if it were even viable in the first place. If you're still against it, feel free to close this out.

@dead-claudia
Copy link
Member Author

I'll close this for now. Let's revisit it if it becomes a real concern.

@dead-claudia dead-claudia deleted the remove-esm branch December 18, 2018 07:29
@dead-claudia dead-claudia mentioned this pull request Feb 1, 2019
Merged
11 tasks
@snyk-bot snyk-bot mentioned this pull request Feb 2, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@dead-claudia dead-claudia

Labels
Type: Bug For bugs and any other unexpected breakage
Projects
Triage/bugs
Status: Closed
Milestone
2.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@dead-claudia @spacejack @barneycarroll