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(core): renderer-to-renderer2 migration not migrating methods #33571
fix(core): renderer-to-renderer2 migration not migrating methods #33571
Conversation
return ` | ||
import '@angular/core'; | ||
declare module "@angular/core" { | ||
class Renderer {} |
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.
We aren't compiling the code so maybe it doesn't matter, but wouldn't this technically cause compilation errors for something like someInstance.setElementClass
since the Renderer
class is empty? Could we do declare const Renderer: any
instead or would that also break type checking?
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.
Yeah it would cause errors, but since we don't collect diagnostics, the error won't be reported. It shouldn't have an effect on the migration either.
I think switching to any
will break since the type checker will then return any
if we call getTypeAtLocation
.
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
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
The `renderer-to-renderer2` migration currently does not work properly in v9 because the migration relies on the type checker for detecting references to `Renderer` from `@angular/core`. This is contradictory since the `Renderer` is no longer exported in v9 `@angular/core`. In order to make sure that the migration still works in v9, and that we can rely on the type checker for the best possible detection, we take advantage of module augmentation and in-memory add the `Renderer` export to the `@angular/core` module.
4d4d711
to
7fd552d
Compare
This comment has been minimized.
This comment has been minimized.
Caretaker: Failure for |
Presubmit failures are also unrelated. Still need to investigate the integration_test failures |
) The `renderer-to-renderer2` migration currently does not work properly in v9 because the migration relies on the type checker for detecting references to `Renderer` from `@angular/core`. This is contradictory since the `Renderer` is no longer exported in v9 `@angular/core`. In order to make sure that the migration still works in v9, and that we can rely on the type checker for the best possible detection, we take advantage of module augmentation and in-memory add the `Renderer` export to the `@angular/core` module. PR Close #33571
…ular#33571) The `renderer-to-renderer2` migration currently does not work properly in v9 because the migration relies on the type checker for detecting references to `Renderer` from `@angular/core`. This is contradictory since the `Renderer` is no longer exported in v9 `@angular/core`. In order to make sure that the migration still works in v9, and that we can rely on the type checker for the best possible detection, we take advantage of module augmentation and in-memory add the `Renderer` export to the `@angular/core` module. PR Close angular#33571
…ular#33571) The `renderer-to-renderer2` migration currently does not work properly in v9 because the migration relies on the type checker for detecting references to `Renderer` from `@angular/core`. This is contradictory since the `Renderer` is no longer exported in v9 `@angular/core`. In order to make sure that the migration still works in v9, and that we can rely on the type checker for the best possible detection, we take advantage of module augmentation and in-memory add the `Renderer` export to the `@angular/core` module. PR Close angular#33571
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. |
The
renderer-to-renderer2
migration currently does not workproperly in v9 because the migration relies on the type checker
for detecting references to
Renderer
from@angular/core
.This is contradictory since the
Renderer
is no longerexported in v9
@angular/core
. In order to make sure thatthe migration still works in v9, and that we can rely on the
type checker for the best possible detection, we take advantage
of module augmentation and in-memory add the
Renderer
exportto the
@angular/core
module.Before coming up with the augmentation solution, I spent quite some time re-working the
detection to be not based on the
ts.TypeChecker
. Though, I went with this solution as it gives usthe best detection of the
Renderer
and doesn't involve reworking the whole detection.