[LTS fix] only return empty href when LinkTo href generation throws error #19395
Conversation
In 3.24.0-3.24.2 we made changes to let LinkTo href generation returns empty when initial transition is not started. This is not true and causing regression on the super-rental tutorial. In previous tutorials, the LinkTo component will return empty href when `this.owner.setupRouter()` is not called. It generates valid href if `setupRouter()` is called and complete model are passed for dynamic segments. In our previous changes, we removed the requirement to call `setupRouter`, and always return empty href when initial transition is not started. This is not expected when passing complete dynamic segments without initial transition.
xg-wang
changed the title
| try { | ||
| let visibleQueryParams = {}; | ||
| if (queryParams) { | ||
| assign(visibleQueryParams, queryParams); | ||
| this.normalizeQueryParams(routeName, models, visibleQueryParams as QueryParam); | ||
| } | ||
|
|
||
| return router.generate(routeName, ...models, { | ||
| queryParams: visibleQueryParams, | ||
| }); | ||
| } catch (e) { | ||
| // Swallow error when transition has not started. | ||
| // When rendering in tests without visit(), we cannot infer the route context which <LinkTo/> needs be aware of | ||
| if (!router._initialTransitionStarted) { | ||
| return; | ||
| } else { | ||
| throw e; | ||
| } |
There was a problem hiding this comment.
Let's avoid the try/catch when we have started a transition, if we don't do that we break the ability to use the "break on uncaught exception" in the devtools.
I think a plausible solution here is something like:
_generateURL(routeName: string, models: {}[], queryParams: {}) {
}
generateURL(routeName: string, models: {}[], queryParams: {}) {
let router = this.router;
if (router._initialTransitionStarted) {
try {
return this._generateURL(routeName, models, queryParams);
} catch (error) {
return;
}
} else {
return this._generateURL(routeName, models, queryParams);
}
}
|
Very bizarre failure on IE11: I'm going to restart CI as I don't think any code in |
|
@xg-wang @rwjblue Is this what's causing the tutorial build to fail? https://github.com/ember-learn/super-rentals-tutorial/runs/1891443801?check_suite_focus=true#step:11:2684 If so, do you mind triggering a build on the tutorial after this went out to release/beta (Monday probably?) to confirm that it fixed the issue? You can go find the latest cron job that failed (e.g. https://github.com/ember-learn/super-rentals-tutorial/actions/runs/562659205) and click "Re-run jobs". If it won't let you, you can open a PR with an empty or trivial commit to force a build on the PR itself and close it after it ran successfully. /cc @kategengler as well |
Summary
In 3.24.0-3.24.2 we made changes to let LinkTo href generation returns
empty when initial transition is not started. This is not true and
causing regression on the super-rental tutorial ember-learn/super-rentals-tutorial#176.
In previous tutorials, the LinkTo component will return empty href when
this.owner.setupRouter()is not called. It generates valid href ifsetupRouter()is called and complete model are passed for dynamicsegments.
In our previous changes, we removed the requirement to call
setupRouter, and always return empty href when initial transition isnot started. This is not expected when passing complete dynamic segments
without initial transition.
Test done
I linked the fix and
super-rentals-tutorialspass test