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

Support passing an allowStateChange function that is evaluated befo… #154

Merged
merged 12 commits into from
Jun 16, 2023

Conversation

Mtn-View
Copy link
Contributor

@Mtn-View Mtn-View commented Jun 8, 2023

…re a state change

The URL will have already changed at this point, but since allowStateChange is evaluated before the state change has started, we can just set the URL back to what it was before without reloading any states.

…re a state change

The URL will have already changed at this point, but since `allowStateChange` is evaluated before the state change has started, we can just set the URL back to what it was before without reloading any states.
Apparently this could fail before.
@daytonlowell
Copy link
Contributor

What ECMA version are we targeting?

@TehShrike
Copy link
Owner

What ECMA version are we targeting?

looks like I gave myself some cover in the docs by saying "modern JS syntax".

I think nowadays I'd define that as "in major browsers for around 2 years". I assume anyone targeting environments older than that is running babel anyways.

index.js Outdated Show resolved Hide resolved
I added this just so I could test the route guards, but it could come in handy later.
- allowStateChange = false prevents state change (does not start state change, resolve unreachable state, or destroy context of guarded state)
- allowStateChange = true does not prevent state change
- allowStateChange can access domApi

Some of the other examples used the hashRouter as well but I haven't figured out how to write these tests using that, so I'll do that after the weekend
Make sure it only gets called once when we're getting sent to a child case
Only run the route guards if the last completely loaded state and the last state to start activating are the same. This will prevent the route guard from getting tripped again upon a redirect in the destination state.
Copy link
Owner

@TehShrike TehShrike left a comment

Choose a reason for hiding this comment

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

looking pretty good. Though I'm wondering if we should use a name that implies a little more, like allowStateChangeAwayFrom or allowLeaving or something

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
Mtn-View and others added 2 commits June 16, 2023 14:36
Co-authored-by: Josh Duff <me@JoshDuff.com>
- canLeaveState instead of allowStateChange
- Check parameters as well as state names in a couple spots
- Only pass the destination name string, not a state object to allowStateChangeOrRevert
- Array destructuring instead of concat + optional chain + nullish coalescing
@TehShrike TehShrike merged commit 898499d into TehShrike:master Jun 16, 2023
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