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

Remove IE 11 from ES6 modules transpilation targets #16962

Closed
wants to merge 1 commit into from

Conversation

lacolaco
Copy link

@lacolaco lacolaco commented Dec 9, 2021

Issue: #16820

What I did

Remove ie 11 from es6Transpiler targets.

How to test

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

To fix weird behavior described at #16820 , I've tested many approaches in my local environment (technically modifying codes in node_modules). I've not yet created useful repro to validate this solution.

Clearly, this bug is caused when babel-loader tries to parse prettier/standalone.js in es6Transpiler processing. And prettier/standalone.js is truly valid JS file. So, the problem is the incompatibility between prettier/standalone.js and the babel config written in es6Transpiler.

I've tested some patterns of targets in @babel/preset-env options. Currently es6Transpiler uses defaults targets. If I remove only ie 11, errors went away. So I conclude targeting ie 11 is not compatible with prettier/standalone.js.

cc/ @shilman

@nx-cloud
Copy link

nx-cloud bot commented Dec 9, 2021

Nx Cloud Report

CI ran the following commands for commit 347f10f. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.

@kylegach
Copy link
Collaborator

Hi, @lacolaco. Thank you for your contribution! We would not be able to merge this until v7.0, as changing compilation targets is breaking change. In the interim, have you tried the --modern flag, added in v6.3?

@lacolaco
Copy link
Author

lacolaco commented Dec 14, 2021

@kylegach Yes, but no changes by --modern.

I guess the reason is prettier doesn't provide ESM modules by modern entrypoint. https://unpkg.com/browse/prettier@2.5.1/package.json

@stale
Copy link

stale bot commented Jan 9, 2022

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Jan 9, 2022
@stale stale bot removed the inactive label Jan 10, 2022
@MichaelArestad MichaelArestad added patch:yes Bugfix & documentation PR that need to be picked to main branch and removed patch:yes Bugfix & documentation PR that need to be picked to main branch labels Jan 10, 2022
@ndelangen ndelangen assigned yannbf and unassigned kylegach Jan 24, 2022
@ndelangen
Copy link
Member

@yannbf I think you helped in fixing this, is this correct? Mind checking and if so close this PR? @lacolaco thank you for the assist regardless! ❤️

@yannbf
Copy link
Member

yannbf commented Jan 25, 2022

Hey @lacolaco thanks a lot for this PR! Seems like #17239 has fixed this issue already. Can you confirm by trying out the latest Storybook release?

@lacolaco
Copy link
Author

I've confirmed the current release works. Thanks!

@lacolaco lacolaco closed this Jan 25, 2022
@lacolaco lacolaco deleted the fix-issue-16820 branch January 25, 2022 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants