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

[BUGFIX lts] LinkTo with incomplete model failing in rendering tests #19387

Merged
merged 3 commits into from
Feb 10, 2021

Conversation

xg-wang
Copy link
Contributor

@xg-wang xg-wang commented Feb 9, 2021

LinkTo needs route context to allow omitting model from current active
route. Without the guard, tests where LinkTo rendered in tests without
routing transition started will break. See #19364

Add failing test before the fix:
While generating link to route "dynamicWithChild.child": can't access
property "shouldSupercede", newHandlerInfo is undefined

While generating link to route "dynamicWithChild.child": can't access
property "shouldSupercede", newHandlerInfo is undefined
@xg-wang
Copy link
Contributor Author

xg-wang commented Feb 9, 2021

This PR and #19080 should go into 3.24 (LTS)

LinkTo needs route context to allow omitting model from current active
route. Without the guard, tests where LinkTo rendered in tests without
routing transition started will break. See issues/19364
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, can you prefix the commit message with [BUGFIX lts]? That is what let's us know to cherry pick things back into the current LTS (3.24).

packages/@ember/-internals/routing/lib/services/routing.ts Outdated Show resolved Hide resolved
@xg-wang xg-wang changed the title fix: LinkTo with incomplete model failing in rendering tests [BUGFIX lts] LinkTo with incomplete model failing in rendering tests Feb 9, 2021
@xg-wang
Copy link
Contributor Author

xg-wang commented Feb 9, 2021

@rwjblue do I also need to cherry-pick #19326 and title it [BUGFIX lts] and [BUGFIX release]?

@xg-wang xg-wang requested a review from rwjblue February 9, 2021 21:09
@rwjblue
Copy link
Member

rwjblue commented Feb 10, 2021

@rwjblue do I also need to cherry-pick #19326 and title it [BUGFIX lts] and [BUGFIX release]?

No, I'll take care of that.

@rwjblue rwjblue merged commit a8746b9 into emberjs:master Feb 10, 2021
@xg-wang xg-wang deleted the issue-19364 branch February 10, 2021 15:45
@rwjblue
Copy link
Member

rwjblue commented Feb 10, 2021

Done, 3.25.1 is released and 3.24.2 is mid-release.

@rogeraraujo90
Copy link

Hello everyone, regarding this fix, The LinkTo component will not render href anymore during Integration tests?

My Integration test did an assert in the link in the Ember 3.20 that started fail in Ember 3.24.

@lougreenwood
Copy link

@rogeraraujo90 Do you still have a problem here? Seems that I just noticed that <LinkTo @route="bla"> where bla has a dynamic segment doesn't generate an href in integration tests.

@rogeraraujo90
Copy link

@lougreenwood try add this.owner.setupRouter(); in the beforeEach hook!

@lougreenwood
Copy link

lougreenwood commented Nov 24, 2021

Thanks, @rogeraraujo90, did that solve it for you?

I'm also already using setupRouter (and as I understand it, it should be necessary in 3.28 anymore) - seems to be a bug where if there is no @model arg on the linkTo and the linkTo points to a route with dynamic segments, the href isn't added to the linkTo.

I've reproduced by both linking to routes without dynamic segments (href is added) and also adding arbitrary @model="1" to the template being tested, which also works.

The links work fine in acceptance tests and production.

I'm planning on creating a minimal repto tomorrow and finding the release where this gets broken.

@rogeraraujo90
Copy link

Thanks, @rogeraraujo90, did that solve it for you?

I'm also already using setupRouter (and as I understand it, it should be necessary in 3.28 anymore) - seems to be a bug where if there is no @model arg on the linkTo and the linkTo points to a route with dynamic segments, the href isn't added to the linkTo.

I've reproduced by both linking to routes without dynamic segments (href is added) and also adding arbitrary @model="1" to the template being tested, which also works.

The links work fine in acceptance tests and production.

I'm planning on creating a minimal repto tomorrow and finding the release where this gets broken.

Good, go ahead!

I'm in Ember 3.24 yet, and using setupRouter the href is being generated correctly in the integration tests. To be more specific. my project is running 3.24.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants