Make isRouting more reliable + other fixes#442
Merged
ryansolid merged 7 commits intosolidjs:mainfrom Jun 12, 2024
Merged
Conversation
🦋 Changeset detectedLatest commit: 2f05f37 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
355f55c to
a22a432
Compare
Brendonovich
approved these changes
Jun 12, 2024
Contributor
Brendonovich
left a comment
There was a problem hiding this comment.
Tested this inside Mattrax and it seems to be working
Contributor
Author
|
Pulled in your improvements, and added a few minor ones on top |
jpdutoit
commented
Jun 12, 2024
| }; | ||
| } | ||
| }) as CachedFunction<T>; | ||
| }) as unknown as CachedFunction<T>; |
Contributor
Author
There was a problem hiding this comment.
Not related, but needed for newer typescript versions
Member
|
Thanks you both. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
isRoutingto 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 useisRoutingturning 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.