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

TranslatePipe markForCheck - needs to happen on init as well? #61

Closed
NathanWalker opened this issue Mar 16, 2016 · 6 comments
Closed

TranslatePipe markForCheck - needs to happen on init as well? #61

NathanWalker opened this issue Mar 16, 2016 · 6 comments

Comments

@NathanWalker
Copy link
Contributor

The fix works great when using OnPush after a value has changed. However I'm seeing that the pipe does not transform anything when everything initializes/bootstraps from the start.

You can see this on this branch:
https://github.com/NathanWalker/angular2-seed-advanced/tree/on-push

Clone and checkout that branch, then:

npm install
npm start

You'll see that no text is translated on page to start, however if you change the language dropdown in top right, it will then fire change detection and then the pipe will transform and show all the text.

I would submit a PR to fix, but not sure what needs to be done. @ocombe hoping you know :)

@ocombe
Copy link
Collaborator

ocombe commented Mar 20, 2016

So I check, the pipes for menu are called (and it's translated), but the pipes for the content are not called until you type in the input.
The problem is probably on your side, not on the one of this lib :)

@NathanWalker
Copy link
Contributor Author

Interesting. Thanks for checking! I'll see what I can find.

@NathanWalker
Copy link
Contributor Author

Turned out the root component only needed to use ChangeDetectionStrategy.Default. Every other component now uses OnPush and it works fine. Maybe this is to be expected, not sure, but it doesn't look like the root component can be OnPush; not that it needs to be, but seems like it wouldn't matter if whole tree is OnPush including root. Anyhow, solved. Thanks again.

@ocombe
Copy link
Collaborator

ocombe commented Mar 20, 2016

Ok, good to know!

@NathanWalker
Copy link
Contributor Author

NathanWalker commented Jun 23, 2016

This has cropped back up in the latest 2.2.2 release. I'm going to reopen and try to create a plunkr or repo demonstrating the issue later and will post back with a link so you can see.

The workaround does not work with latest release unfortunately.

@ocombe
Copy link
Collaborator

ocombe commented Dec 21, 2016

I'm using it with OnPush at work without any problem, I think I can close this. Let me know if you still have a problem and I'll reopen.

@ocombe ocombe closed this as completed Dec 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Bugs
Accepted. To fix
Development

No branches or pull requests

2 participants