Skip to content

Make isRouting more reliable + other fixes#442

Merged
ryansolid merged 7 commits intosolidjs:mainfrom
jpdutoit:is-routing-and-intent
Jun 12, 2024
Merged

Make isRouting more reliable + other fixes#442
ryansolid merged 7 commits intosolidjs:mainfrom
jpdutoit:is-routing-and-intent

Conversation

@jpdutoit
Copy link
Contributor

@jpdutoit jpdutoit commented Jun 8, 2024

I set out to fix #311, but in doing so discovered a few other issues

The isRouting value was not very reliable, and could change many times between true and false

This PR changes isRouting to update once to true when routing starts, and then back to false when routing ends - but only after the source has been updated. The ordering will allow one to use isRouting turning false as a trigger for when it is safe to update your document title. I added tests for these.

While doing this I discovered and fixed the following issues:

Question:
I noticed that a transition was started on creation. Is that initial transition important? I currently have it commented out.

@changeset-bot
Copy link

changeset-bot bot commented Jun 8, 2024

🦋 Changeset detected

Latest commit: 2f05f37

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@solidjs/router Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@Brendonovich Brendonovich left a comment

Choose a reason for hiding this comment

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

Tested this inside Mattrax and it seems to be working

@jpdutoit
Copy link
Contributor Author

Pulled in your improvements, and added a few minor ones on top

};
}
}) as CachedFunction<T>;
}) as unknown as CachedFunction<T>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not related, but needed for newer typescript versions

@ryansolid
Copy link
Member

Thanks you both.

@ryansolid ryansolid merged commit 10b7d8c into solidjs:main Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants