-
Notifications
You must be signed in to change notification settings - Fork 11.9k
feat(@ngtools/webpack): replace server bootstrap code #5194
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
|
@FrozenPandaz can you add a test that shows these changes work? A good place to add would be side by side with the other webpack plugin tests in https://github.com/angular/angular-cli/tree/master/tests/e2e/tests/packages/webpack These tests will create a new project from the files in https://github.com/angular/angular-cli/tree/master/tests/e2e/assets/webpack. You'll need a new project in the last folder, and a new file that tests it in the first. |
|
@filipesilva yes i will add tests, thanks for pointing me to the right place i was actually confused about that because theres a loader spec which looks quite empty. |
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
1c8ceaf to
3490c18
Compare
|
CLAs look good, thanks! |
407fde0 to
d967b51
Compare
|
@filipesilva I've added tests. Please take a look. |
filipesilva
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job on the test setup! I'm requesting Hans's review on this as the authority on the plugin.
9304bb7 to
24a6914
Compare
|
I rebased this again. @hansl |
24a6914 to
f8aea74
Compare
83db000 to
854f960
Compare
|
Could you provide some updates about this issue ? |
|
Anything new regarding this PR? It is extremely important for our project. |
a4894bf to
e61fae4
Compare
1ac4d48 to
497f5b1
Compare
|
CLAs look good, thanks! |
9a7716f to
016445e
Compare
7aa110f to
31774d7
Compare
|
@hansl This should be ready for you to take a look |
hansl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
feat(@ngtools/webpack): replace server bootstrap code (angular#5194)
feat(@ngtools/webpack): replace server bootstrap code (angular#5194)
|
Saw it merged to master, any ideas when we're releasing to npm? |
|
Is this depends on #5547? |
|
Nope this does not depend on #5547 |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Currently the loader is only replacing the bootstrap code of platformBrowser.
It should replace the bootstrap code of platformServer as well.
I also made it expandable to other platforms. We should probably add webworker platform as well.
I can do that in a separate pull request?
Motivation for this is better support of platform-server via @ngtools/webpack