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

Update babel plugins and pin to @babel/[email protected] #385

Merged
merged 3 commits into from
Mar 10, 2021

Conversation

patocallaghan
Copy link
Contributor

@patocallaghan patocallaghan commented Mar 7, 2021

Upgrading all babel-plugin-* dependencies due to conversation in #382 (comment)

  • @babel/plugin-proposal-decorators
  • @babel/plugin-transform-modules-amd
  • @babel/plugin-transform-runtime
  • @babel/plugin-transform-typescript
  • babel-plugin-ember-modules-api-polyfill

It also pins @babel/runtime to 7.12.18 to workaround test failures in CI for the "Floating dependencies" scenario.

/cc @rwjblue

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

Can you also update package.json to use those new minimum versions as well?

@patocallaghan patocallaghan force-pushed the patoc/update-babel-plugins branch from 218380c to 61fce1c Compare March 8, 2021 09:37
@patocallaghan
Copy link
Contributor Author

Updated the package.json 👍

@rwjblue
Copy link
Member

rwjblue commented Mar 8, 2021

Hmmm. Something is causing the floating dependencies tests to time out.

@patocallaghan
Copy link
Contributor Author

@rwjblue seeing this in CI. Will have a dig later

image

@rwjblue
Copy link
Member

rwjblue commented Mar 8, 2021

Seems like the same thing being reported in later comments over in #384

@patocallaghan
Copy link
Contributor Author

Interesting, so bumping a few other babel-related deps has caused the issue to manifest in the non-"floating dependencies" tests. Unfortunately it's only happening in CI for me, I can't reproduce locally, so will need to rollback each of those upgrades I just did one-by-one.

image

@rwjblue
Copy link
Member

rwjblue commented Mar 8, 2021

I've canceled some of the CI runs (to try to get faster turn around)

@patocallaghan patocallaghan force-pushed the patoc/update-babel-plugins branch from bb92148 to 6f1f042 Compare March 9, 2021 20:57
@patocallaghan patocallaghan force-pushed the patoc/update-babel-plugins branch from 6f1f042 to 7ceada3 Compare March 9, 2021 21:04
@patocallaghan
Copy link
Contributor Author

patocallaghan commented Mar 9, 2021

@rwjblue it's not pretty but I've pinned @babel/runtime to 7.12.18 and that resolves the floating dependencies issue. I haven't bottomed-out out on it yet but it appears to be related to babel/babel#12881. This issue was closed but bumping to the latest babel/runtime caused it to still happen.

Unfortunately as I stated earlier, I can only reproduce the issue on CI which makes debugging a bit painful.

@boris-petrov
Copy link

FWIW I also had to pin @babel/runtime in my app to 7.12.18 but only because of an "old" targets.js. Using:

[
  'chrome >= 85',
  'edge >= 85',
  'firefox >= 80',
  'safari >= 14',
]

Works fine with @babel/runtime 7.13.9+ but using ['firefox 35'] for targets leads to the error and I have to use 7.12.18 because of that. Just to mention it, not sure if it's relevant here.

@patocallaghan
Copy link
Contributor Author

Thanks @boris-petrov, that could well be the case here as we add ie11 in CI.

https://github.com/babel/ember-cli-babel/blob/20924faa529cf4f9c6f7cb8fa2c89d968184b620/tests/dummy/config/targets.js#L3-L14

I guess we should keep it pinned though as officially the Ember framework still supports IE11 (until 4.0 anyway)?

@boris-petrov
Copy link

Yep, that seems to be the same issue as mine. Well, yes, for now it's better to pin. I will be trying from time to time to update and I'll be sure to let you know if a 7.13.x or later version works so you can unpin. Or you can find and fix the underlying cause. 😄 I'm not sure where it could be though.

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

Successfully merging this pull request may close these issues.

3 participants