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 ember-cli-babel to prevent errors with newer TS syntax #7220

Merged
merged 1 commit into from
Jun 9, 2020

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Jun 9, 2020

NOTE: Targets the 3.16 branch (as this no longer affects the current release).

@ember-data/store@3.16 introduced new typescript syntax that is not guaranteed to be supported by the ember-cli-babel version that it depends on. Specifically, the version of ember-cli-babel we depend on prior to this change uses @babel/parser@7.3.3 which does not support the function foo(id: string | null) asserts id is not null {} syntax we use in fetch-manager.ts. The dependency structure looks like this (with links):

The specific assertion syntax was not parsable until @babel/parser@7.7.0 (added by babel/babel@38a3063).

Without the changes added here, you would be able to update to ember-data@3.16.6 and still have an older version of @babel/parser that was incapable of parsing the syntax used by ember-data@3.16. This would result in the following error message:

SyntaxError: app-path/-private/system/fetch-manager.ts: Unexpected token, expected "{" (506:52)

  504 | }
  505 | 
> 506 | function assertIsString(id: string | null): asserts id is string {
      |                                                     ^
  507 |   if (DEBUG) {
  508 |     if (typeof id !== 'string') {
  509 |       throw new Error(`Cannot fetch record without an id`);
    at Object.raise (app-path/node_modules/@ember-data/store/node_modules/broccoli-babel-transpiler/node_modules/@babel/parser/lib/index.js:6322:17)
    at Object.unexpected (app-path/node_modules/@ember-data/store/node_modules/broccoli-babel-transpiler/node_modules/@babel/parser/lib/index.js:7638:16)
    at Object.expect (app-path/node_modules/@ember-data/store/node_modules/broccoli-babel-transpiler/node_modules/@babel/parser/lib/index.js:7624:28)
    at Object.parseBlock (app-path/node_modules/@ember-data/store/node_modules/broccoli-babel-transpiler/node_modules/@babel/parser/lib/index.js:10305:10)
    at Object.parseFunctionBody (app-path/node_modules/@ember-data/store/node_modules/broccoli-babel-transpiler/node_modules/@babel/parser/lib/index.js:9382:24)
    at Object.parseFunctionBodyAndFinish (app-path/node_modules/@ember-data/store/node_modules/broccoli-babel-transpiler/node_modules/@babel/parser/lib/index.js:9352:10)
    at Object.parseFunctionBodyAndFinish (app-path/node_modules/@ember-data/store/node_modules/broccoli-babel-transpiler/node_modules/@babel/parser/lib/index.js:5234:11)
    at withTopicForbiddingContext (app-path/node_modules/@ember-data/store/node_modules/broccoli-babel-transpiler/node_modules/@babel/parser/lib/index.js:10474:12)
    at Object.withTopicForbiddingContext (app-path/node_modules/@ember-data/store/node_modules/broccoli-babel-transpiler/node_modules/@babel/parser/lib/index.js:9657:14)
    at Object.parseFunction (app-path/node_modules/@ember-data/store/node_modules/broccoli-babel-transpiler/node_modules/@babel/parser/lib/index.js:10473:10)

`@ember-data/store@3.16` introduced new typescript syntax that is not
guaranteed to be supported by the ember-cli-babel version that it
depends on. Specifically, the version of ember-cli-babel we depend on
prior to this change uses `@babel/parser@7.3.3` which does not support
the `function foo(id: string | null) asserts id is not null {}` syntax
we use in `fetch-manager.ts`. The dependency structure looks like this
(with links):

* [ember-cli-babel@7.13.2 depends on broccoli-babel-transpiler@7.3.0](https://github.com/babel/ember-cli-babel/blob/v7.13.2/package.json#L55)
* [broccoli-babel-transpiler@7.3.0 depends on @babel/core@7.3.3](https://github.com/babel/broccoli-babel-transpiler/blob/v7.3.0/package.json#L39)
* [@babel/core@7.3.3 depends on @babel/parser@7.3.3](https://github.com/babel/babel/blob/v7.3.3/packages/babel-core/package.json#L39)

The specific assertion syntax was not parsable until
`@babel/parser@7.7.0` (added by
babel/babel@38a3063).
@rwjblue rwjblue added the Bug label Jun 9, 2020
@stefanpenner
Copy link
Member

@runspired runspired added 🎯 beta PR should be backported to beta 🎯 lts The PR should be backported to the most recent LTS 🎯 release PR should be backported to release labels Jun 9, 2020
@runspired runspired added backport-lts PR targets the current lts branch and removed 🎯 beta PR should be backported to beta 🎯 lts The PR should be backported to the most recent LTS 🎯 release PR should be backported to release labels Jun 9, 2020
@igorT
Copy link
Member

igorT commented Jun 9, 2020

Github actions passed, so merging. Thanks a bunch for the fix!

@igorT igorT merged commit 43cf4d3 into lts-3-16 Jun 9, 2020
@delete-merged-branch delete-merged-branch bot deleted the update-minimum-ember-cli-babel branch June 9, 2020 20:54
@stefanpenner
Copy link
Member

stefanpenner commented Jun 9, 2020

@igorT / @runspired No trying to rush, just curious for planning purposes. Do we have an ETA for release?

@igorT
Copy link
Member

igorT commented Jun 9, 2020 via email

@runspired runspired added 🏷️ bug This PR primarily fixes a reported issue and removed Bug labels Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-lts PR targets the current lts branch 🏷️ bug This PR primarily fixes a reported issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants