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

fix(@schematics/angular): handle aliased or existing environment import #16377

Merged
merged 1 commit into from
Dec 21, 2019
Merged

fix(@schematics/angular): handle aliased or existing environment import #16377

merged 1 commit into from
Dec 21, 2019

Conversation

arturovt
Copy link
Contributor

@arturovt arturovt commented Dec 6, 2019

Closes: #16226

The exception Cannot read property 'length' of undefined wasn't really informative thus I had to find out where this exception was actually thrown. All ways led me to the Chunk class and the insertLeft method that used buffer's variable length property. The buffer was undefined, that's why that exception was thrown. It was undefined because there is no way to check if environment is already imported.

This PR adds the ability to handle the already existing "environment" import or the aliased one.

@arturovt arturovt changed the title fix(@schematics/angular): handle named environment import fix(@schematics/angular): handle aliased or existing environment import Dec 6, 2019
@clydin clydin added the target: patch This PR is targeted for the next patch release label Dec 11, 2019
// Find specifier that contains `environment` keyword in its text.
// Whether it's `environment` or `environment as env`.
const nodes = findNodes(declaration, ts.SyntaxKind.ImportSpecifier).filter(node =>
node.getText().includes('environment'),
Copy link
Member

Choose a reason for hiding this comment

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

getText can be an expensive operation.
Can you move the named bindings analysis up to this point and use it to check the values?

@clydin
Copy link
Member

clydin commented Dec 17, 2019

Can you rebase on master? This should fix the failing tests.

@arturovt
Copy link
Contributor Author

arturovt commented Dec 17, 2019

Can you rebase on master? This should fix the failing tests.

Done.

@mgechev mgechev merged commit 5458477 into angular:master Dec 21, 2019
@arturovt arturovt deleted the fix/service-worker-named-import branch December 21, 2019 08:39
@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 Feb 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ng add @angular/pwa - Cannot read property 'length' of undefined
4 participants