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

Migrating from 6.1 to 7.2 does not remove/move reflect metadata polyfill #13491

Closed
Astrophizz opened this issue Jan 21, 2019 · 6 comments · Fixed by #13528
Closed

Migrating from 6.1 to 7.2 does not remove/move reflect metadata polyfill #13491

Astrophizz opened this issue Jan 21, 2019 · 6 comments · Fixed by #13528
Assignees
Labels
freq1: low Only reported by a handful of users who observe it rarely severity3: broken
Milestone

Comments

@Astrophizz
Copy link

Astrophizz commented Jan 21, 2019

🐞 Bug report

Command (mark with an x)

- [ ] new
- [ ] build
- [ ] serve
- [ ] test
- [ ] e2e
- [ ] generate
- [ ] add
- [x] update
- [ ] lint
- [ ] xi18n
- [ ] run
- [ ] config
- [ ] help
- [ ] version
- [ ] doc

Is this a regression?

No.

Description

According to the blog post about Angular 7, updating to Angular 7 should remove the reflect metadata polyfill. The work in #12314, #12611, and #12625 suggests it might also get moved the the dev environment, though discussions around the code seem to disagree. No changes are made to polyfill.ts when updating.

🔬 Minimal Reproduction

Run ng update --all --force on an Angular 6 application (this works around other outstanding issues with ng update).

🌍 Your Environment


     _                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/
    

Angular CLI: 6.2.5
Node: 10.15.0
OS: win32 x64
Angular: 6.1.10
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.6.8
@angular-devkit/build-angular     0.6.8
@angular-devkit/build-optimizer   0.6.8
@angular-devkit/core              0.6.8
@angular-devkit/schematics        0.6.8
@angular/cli                      6.2.5
@ngtools/webpack                  6.0.8
@schematics/angular               0.8.5
@schematics/update                0.8.5
rxjs                              6.3.3
typescript                        2.9.2
webpack                           4.8.3

Anything else relevant?

Not that I can think of. It might be nice for the Angular Update Guide to explain how to manually migrate the polyfill if the automated migration does not do anything.

@alan-agius4
Copy link
Collaborator

Hi, I tried to replicate this and the import 'core-js/es7/reflect'; was removed.

Can you setup a minimal repro please?

This might be related to your directory structure so its really important to get an accurate repro to diagnose this.

Thanks

@alan-agius4 alan-agius4 added the needs: repro steps We cannot reproduce the issue with the information given label Jan 22, 2019
@Astrophizz
Copy link
Author

Astrophizz commented Jan 23, 2019

In attempting to create a minimal repro by amputating my application until it resembled a fresh Angular project and the migration still failing to remove the polyfill, I think I've narrowed in on the difference between a successful migration and one that doesn't remove the reflect polyfill. The key difference was whether I ran ng update --all without a pre-existing .\node_modules (success) or running npm i to populate .\node_modules before ng update --all (failure). Here's a repo where you can try it yourself: https://github.com/Astrophizz/angular-6-to-7-reflect-polyfill

On a side note, I do find it odd that the migration removes the polyfill import but not the comment describing it.

@alan-agius4 alan-agius4 added freq1: low Only reported by a handful of users who observe it rarely severity3: broken comp: schematics/update and removed needs: repro steps We cannot reproduce the issue with the information given labels Jan 24, 2019
@ngbot ngbot bot added this to the needsTriage milestone Jan 24, 2019
@alan-agius4 alan-agius4 added the needs: investigation Requires some digging to determine if action is needed label Jan 24, 2019
@alan-agius4 alan-agius4 self-assigned this Jan 25, 2019
@alan-agius4
Copy link
Collaborator

@Astrophizz, the problem seems to be caused because of an older version of @angular-devkit/build-angular. Kindly update this to at least 0.8.0.

Also you don't need to depending on these packages directly as this might cause undefined behaviour.

    "@angular-devkit/core": "^0.6.5",
    "@angular-devkit/schematics": "^0.6.5",

in https://github.com/Astrophizz/angular-6-to-7-reflect-polyfill/blob/11352852849fdae77d304d6a5d8e9eba7937d3c3/package.json#L44-L45

@alan-agius4 alan-agius4 removed the needs: investigation Requires some digging to determine if action is needed label Jan 27, 2019
@Astrophizz
Copy link
Author

Thanks for finding that and for the guidance!


The rest of this comment may be more useful to anyone who stumbles across this in the future.

Those references to @angular-devkit/* were added when I updated @ngrx/schematics from version 5 to 6. It seems that @angular-devkit/core and @angular-devkit/schematics were peer dependencies for @ngrx/schematics back then (in June 2018) and somehow got added to the devDependencies. The commits were squashed and I don't recall if I added them to devDependencies to address a peer dependency warning. Fortunately those peer dependencies have since been removed, so I expect I'll be able to make the changes you suggested without breaking @ngrx/schematics 🙂

hansl pushed a commit that referenced this issue Jan 30, 2019
hansl pushed a commit that referenced this issue Jan 30, 2019
hansl pushed a commit that referenced this issue Jan 30, 2019
@Slash7GNR
Copy link

Slash7GNR commented Mar 15, 2019

Hey,
I'm encountering the same issue using @angular-devkit/build-angular version ~0.12.0, anyone knows why ?

@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
freq1: low Only reported by a handful of users who observe it rarely severity3: broken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants