Skip to content

fix(schematics): incorrectly throws if NgModule uses namespaced decorator #15298

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

Currently if the developer defines the primary app NgModule using a namespaced
import (e.g. @ngCore.NgModule), the Material schematics will incorrectly not detect
that module as the logic that resolves the decorator name of a class declaration
is flawed.

This is because it incorrectly traverses a PropertyAccessExpression, while the
accessed property name is exposed at the root level PropertyAccessExpression.

This PR fixes this and also adds additional unit tests for the hasNgModuleImport
utility function. (We have integration tests for this; but we don't test that special case)

…ator

Currently if the developer defines the primary app `NgModule` using a namespaced
import (e.g. "@ngCore.NgModule`), the Material schematics will incorrectly not detect
that module as the logic that resolves the decorator name of a class declaration
is flawed.

This is because it incorrectly traverses a `PropertyAccessExpression`, while the
accessed property name is exposed at the root level `PropertyAccessExpression`.

This commit fixes this and also adds additional unit tests for the `hasNgModuleImport`
utility function. (We have integration tests for this; but we don't test that special case)
@devversion devversion added pr: merge safe target: patch This PR is targeted for the next patch release labels Feb 25, 2019
@devversion devversion requested a review from jelbourn as a code owner February 25, 2019 17:25
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Feb 25, 2019
@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Feb 25, 2019
@josephperrott josephperrott merged commit da8987b into angular:master Mar 4, 2019
josephperrott pushed a commit that referenced this pull request Mar 4, 2019
…ator (#15298)

Currently if the developer defines the primary app `NgModule` using a namespaced
import (e.g. "@ngCore.NgModule`), the Material schematics will incorrectly not detect
that module as the logic that resolves the decorator name of a class declaration
is flawed.

This is because it incorrectly traverses a `PropertyAccessExpression`, while the
accessed property name is exposed at the root level `PropertyAccessExpression`.

This commit fixes this and also adds additional unit tests for the `hasNgModuleImport`
utility function. (We have integration tests for this; but we don't test that special case)
@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 10, 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: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants