-
Notifications
You must be signed in to change notification settings - Fork 26.2k
feat: remove deprecated DOCUMENT token from platform-browser #28117
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
3773319
to
c8b85a1
Compare
c8b85a1
to
2e106f1
Compare
cc @IgorMinar The saucelabs failure is a flake and just needs to be restarted. |
A refactor doesn't change behaviour or break compatibility. Behaviour is being changed here and there's nothing being fixed, so I guess this is a |
@alfaproject Yeah I don't know. It's literally the opposite of a |
I expect that this will break a significant amount of code at google. I'm running a presubmit now to verify the impact. If confirmed, we might need to create a ts-lint refactoring transform to make this change automatically and roll this out as part of @StephenFluin can we get a reliable estimate of this change on external users? I'm quite sure it's significant and therefore we could roll this out only with automated refactoring of apps and libraries (which in this case is safe and relatively straightforward). |
internal presubmit results: http://test/OCL:229126105:BASE:229126324:1547447183003:cc4ee4a9 preliminary results: there is a handful of apps that still work after this change, but great majority of apps in google3 are broken. this means that we'll need the ts-lint transform similar to several we built for rxjs in order to roll this change out. @CaerusKaru are you up for taking that on? Now in order to make the transform compatible with We then write a |
and yes, "refactor" is misleading. we might need a new category for this type of change, but in the meantime use "feat" as weird as it is because it's best category in terms of user visibility of the change in the changelog. |
Around 80% of apps I just scanned use the I wouldn't have guessed that it would be many people, as I would guess most developers access |
48c338a
to
60d1f79
Compare
Blocked on #29237 |
Also blocked on angular/components#15470 |
60d1f79
to
05b02a9
Compare
05b02a9
to
6436cdb
Compare
6436cdb
to
41e1218
Compare
bump @alexeagle @IgorMinar This can be merged now that #29950 has been merged |
41e1218
to
30ea564
Compare
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. |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
DOCUMENT
is a token exported by both@angular/platform-browser
(deprecated) and@angular/common
Issue Number: N/A
What is the new behavior?
DOCUMENT
is no longer a token exported from@angular/platform-browser
. It is now only exported from@angular/common
Does this PR introduce a breaking change?