Skip to content

refactor(schematics): avoid typescript version conflicts #12989

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

Conversation

devversion
Copy link
Member

The Angular Material schematics parse TypeScript sources files and pass the AST to the @schematics/angular package which defined an explicit dependency on typescript. Since we currently just always require the flattened typescript dependency (node_modules/typescript), there could be either no TypeScript version or a different TypeScript version that causes the AST operations of the @schematics/angular utility functions to not work properly (e.g. different SyntaxKind ids)

We should primarily try to load the same TypeScript version that has been shipped with the @schematics/angular. If that one couldn't be found, fall back to the top level typescript version.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Sep 5, 2018
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added target: major This PR is targeted for the next major release pr: lgtm action: merge The PR is ready for merge by the caretaker labels Sep 5, 2018
@devversion devversion force-pushed the refactor/schematics-typescript-version-conflicts branch 2 times, most recently from a00e56b to 99db7ed Compare September 5, 2018 19:32
@devversion devversion removed the action: merge The PR is ready for merge by the caretaker label Sep 5, 2018
@devversion devversion force-pushed the refactor/schematics-typescript-version-conflicts branch from 7d07856 to 99db7ed Compare September 5, 2018 20:23
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added the action: merge The PR is ready for merge by the caretaker label Sep 5, 2018
The Angular Material schematics parse TypeScript sources files and pass the AST to the `@schematics/angular` package which defined an explicit dependency on `typescript`. Since we currently just always require the flattened `typescript` dependency (`node_modules/typescript`), there could be either no TypeScript version or a different TypeScript version that causes the AST operations of the `@schematics/angular` utility functions to not work properly (e.g. different `SyntaxKind` ids)

We should primarily try to load the same TypeScript version that has been shipped with the `@schematics/angular`. If that one couldn't be found, fall back to the top level `typescript` version.
@devversion devversion force-pushed the refactor/schematics-typescript-version-conflicts branch from 98b7df8 to 33cb34c Compare September 9, 2018 10:50
@jelbourn jelbourn merged commit 9886b1b into angular:master Sep 10, 2018
@devversion devversion deleted the refactor/schematics-typescript-version-conflicts branch September 10, 2018 14:21
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants