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

fix(ivy): support ICUs with pipes #34198

Closed
wants to merge 1 commit into from

Conversation

@AndrewKushnir
Copy link
Contributor

AndrewKushnir commented Dec 2, 2019

Prior to this commit, i18n runtime code failed with the exception saying that no provider was found for ChangeDetectorRef for a pipe used in ICU. The problem happened because the underlying createViewRef function was not taking into account IcuContainer as a valid TNodeType. This commit updates the createViewRef function to return corresponding ViewRef for TNodeType.IcuContainer.

This PR resolves #34159.

PR Type

What kind of change does this PR introduce?

  • Bugfix

Does this PR introduce a breaking change?

  • Yes
  • No
Prior to this commit, i18n runtime code failed with the exception saying that no provider was found for ChangeDetectorRef for a pipe used in ICU. The problem happened because the underlying `createViewRef` function was not taking into account IcuContainer as a valid TNodeType. This commit updates the `createViewRef` function to return corresponding ViewRef for TNodeType.IcuContainer.
@ngbot ngbot bot added this to the needsTriage milestone Dec 2, 2019
@googlebot googlebot added the cla: yes label Dec 2, 2019
@AndrewKushnir AndrewKushnir marked this pull request as ready for review Dec 2, 2019
@AndrewKushnir AndrewKushnir requested a review from angular/fw-core as a code owner Dec 2, 2019
Copy link
Member

pkozlowski-opensource left a comment

LGTM from the runtime point of view.

I'm still feeling super-uncomfortable with the fact that for ICU containers we start injection with a parent of a text node, but based on our last discussion this sounds like the expected behaviour...

@AndrewKushnir

This comment has been minimized.

Copy link
Contributor Author

AndrewKushnir commented Dec 3, 2019

Thanks for review @pkozlowski-opensource 👍

@AndrewKushnir

This comment has been minimized.

Copy link
Contributor Author

AndrewKushnir commented Dec 3, 2019

mhevery added a commit that referenced this pull request Dec 4, 2019
Prior to this commit, i18n runtime code failed with the exception saying that no provider was found for ChangeDetectorRef for a pipe used in ICU. The problem happened because the underlying `createViewRef` function was not taking into account IcuContainer as a valid TNodeType. This commit updates the `createViewRef` function to return corresponding ViewRef for TNodeType.IcuContainer.

PR Close #34198
@mhevery mhevery closed this in 60b13d9 Dec 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.