Change urlRerouteOnly default value to true. - #828
Conversation
File size impactdist (+712 bytes)Overall impact on dist files size
Detailed impact on dist files size
Impact on dist files cache8 files in you users cache are now outdated because their content have changed.
|
| history.replaceState({ some: "state" }, document.title); | ||
| // calling triggerAppChange forcibly increments the counters which is weird for this test | ||
| // but it also ensures we wait for the reroute to finish (if it's taking place) | ||
| it(`Fires artificial popstate events with correct target`, async () => { |
There was a problem hiding this comment.
I think you can do aysnc (done) here if that makes thing easier.
There was a problem hiding this comment.
async (done) is being removed, according to https://jestjs.io/blog/2021/05/25/jest-27#features-coming-with-breaking-changes
| export function start(opts) { | ||
| started = true; | ||
| if (opts && opts.urlRerouteOnly) { | ||
| if (opts) { |
There was a problem hiding this comment.
Is it possible that !!opts === true && opts.urlRerouteOnly === undefined ? Does that need to be checked?
There was a problem hiding this comment.
Yes, but it's okay if urlRerouteOnly is undefined, since it's just used for a truthiness check and will count as false. For a while, I was doing opts.hasOwnProperty("urlRerouteOnly"), but figured it wasn't worth doing that since there's no reason to provide options except to set urlRerouteOnly.
I guess it might be confusing though for start() to behave differently from start({}). I'll change it.
|
Going to merge this now, as feedback was addressed. |
2 participants
When urlRerouteOnly is true, single-spa does not reroute, fire extraneous popstate events, etc when the URL does not change during a navigation.
The option was introduced in a minor version of single-spa@5, but turned off. However, I think it's a good default to have it on, as it's a good performance boost for those following best practices of using the URL as the thing that determines whether a reroute is needed.
This is a breaking change that I'm thinking will be part of 6.0