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

feat: optimize get fragment visit #79

Merged
merged 15 commits into from
Jun 4, 2024
Merged

Conversation

hirasso
Copy link
Member

@hirasso hirasso commented Jun 1, 2024

Description

I just stumbled upon a case where the sole fragment that was being targeted for the current visit already had the same fragment url as visit.to.url, causing a normal (non-fragment) visit that broke as of course the containers were missing (hard to explain 😅). Allowing visits that don't replace anything was exactly what I needed in this case.

Actually, the comment I posted below lead me to the real solution: Treat a visit as a reload, if all fragment elements already match that visit's visit.to.url. In other words: Don't skip fragments as described in https://swup.js.org/plugins/fragment-plugin/#how-fragment-containers-are-found, bullet 3, if otherwise no fragments would be replaced at all.

  • includes Version bump and changelog update

Drive-By

  • The visit introduced for rule.if had the wrong route if being called as an API function. It will now be created on the fly using swup.createVisit(route).

Checks

  • The PR is submitted to the master branch
  • The code was linted before pushing (npm run lint)
  • All tests are passing (npm run test)
  • New or updated tests are included
  • The documentation was updated as required

Additional information

@hirasso hirasso requested a review from daun June 1, 2024 18:06
Copy link

github-actions bot commented Jun 1, 2024

Playwright test results

passed  12 passed

Details

stats  12 tests across 1 suite
duration  
commit  6b6d10d

@hirasso
Copy link
Member Author

hirasso commented Jun 1, 2024

An alternative could be to check if all containers for the current visit already match the visit.to.url, and if so, replace them anyways. @daun Maybe this is something we should discuss in a call.

@hirasso hirasso closed this Jun 2, 2024
@hirasso
Copy link
Member Author

hirasso commented Jun 2, 2024

Closing this as I'd like to go through all this in an issue first.

@hirasso hirasso reopened this Jun 4, 2024
@hirasso hirasso merged commit bf0c98b into master Jun 4, 2024
6 checks passed
@hirasso hirasso deleted the feat/optimize-get-fragment-visit branch June 4, 2024 11:03
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.

2 participants