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

chore: angular 4 upgrade #1415

Closed
wants to merge 13 commits into from
Closed

Conversation

willyelm
Copy link
Contributor

@willyelm willyelm commented Mar 28, 2017

Before submitting a pull request, please make sure you have at least performed the following:

  • read and followed the CONTRIBUTING.md guide.
  • built and tested the changes locally.
  • added/updated any applicable tests.
  • added/updated any applicable API documentation.
  • added/updated any applicable demos.

@pkozlowski-opensource
Copy link
Member

Hi! Thnx for the PR. We can't merge it just yet as it would mean dropping support for Angular 2.x and we don't want to do it just yet. See #1337 for more details.

Having said this I'm going to keep this PR around so we know what / where to change when the update time comes.

@andonyns
Copy link

andonyns commented Apr 1, 2017

@pkozlowski-opensource Have you guys considered moving the v4 migration to a different branch and make both available through npm so that people can use it both on angular 2 and 4? As the Angular folks do it.

"target": "ES5",
"lib": ["es2015", "dom"],
"target": "es5",
"lib": ["es2016", "dom"],

Choose a reason for hiding this comment

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

Why this change?

"module": "commonjs",
"moduleResolution": "node",

Choose a reason for hiding this comment

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

Why this change?

require('zone.js/dist/proxy.js');
require('zone.js/dist/sync-test');
require('zone.js/dist/async-test');
require('zone.js/dist/fake-async-test');

Choose a reason for hiding this comment

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

I don't think we use those for unit tests so we shouldn't be having this change.

@pkozlowski-opensource
Copy link
Member

@willyelm thnx for the PR - I've just started to review it but there are many changes in there and many of those are not related to the Angular 4 switch... Given this (and the fact that this PR doesn't merge cleanly any more) I'm going to pick up bits and pieces but won't be able to land the whole PR. I will leave comments here indicating which commits I'm brining in. Once again, thnx for the PR.

@willyelm
Copy link
Contributor Author

Hi @pkozlowski-opensource, Thanks! let me know if I can help.

pkozlowski-opensource pushed a commit that referenced this pull request Apr 22, 2017
@pkozlowski-opensource
Copy link
Member

Merged WebPack config changes as 6616c77 - it was possible to land those now since not related to Angular 4 switch.

lupinthethirdgentleman pushed a commit to lupinthethirdgentleman/angular-bootstrap that referenced this pull request Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants