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

Fix double before-fetch-request dispatch during reload #740

Merged
merged 1 commit into from
Sep 27, 2022

Conversation

seanpdoyle
Copy link
Contributor

Closes #739

Issue #739 mentions the issue at the root of the unexpected behavior:

The reason is that reload() calls this.removeAttribute("complete")
and the frame watches attribute changes and calls
this.loadSourceURL() when the “complete” attribute is removed.

This commit resolves the issue in several steps.

First, add the sourceURLReloaded() callback to the FrameElementDelegate interface.

Next, move the FrameElement.reload() implementation from FrameElement.reload() to the FrameController.sourceURLReloaded().

Finally, wrap the implementation's call to
this.element.removeAttribute("complete") within a ignoringChangesToAttribute("complete", () => ...) block so that the change to the attribute is temporarily ignored.

Closes hotwired#739

Issue hotwired#739 mentions the issue at the root of the unexpected behavior:

> The reason is that `reload()` calls `this.removeAttribute("complete")`
> and the frame watches attribute changes and calls
> `this.loadSourceURL()` when the “complete” attribute is removed.

This commit resolves the issue in several steps.

First, add the `sourceURLReloaded()` callback to the
`FrameElementDelegate` interface.

Next, move the `FrameElement.reload()` implementation from
`FrameElement.reload()` to the `FrameController.sourceURLReloaded()`.

Finally, wrap the implementation's call to
`this.element.removeAttribute("complete")` within a
`ignoringChangesToAttribute("complete", () => ...)` block so that the
change to the attribute is temporarily ignored.
@dhh dhh merged commit 732db00 into hotwired:main Sep 27, 2022
@seanpdoyle seanpdoyle deleted the fix-issue-739 branch September 27, 2022 14:29
another-rex referenced this pull request in google/osv.dev Oct 31, 2022
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@hotwired/turbo](https://turbo.hotwired.dev)
([source](https://togithub.com/hotwired/turbo)) | [`7.2.0` ->
`7.2.4`](https://renovatebot.com/diffs/npm/@hotwired%2fturbo/7.2.0/7.2.4)
|
[![age](https://badges.renovateapi.com/packages/npm/@hotwired%2fturbo/7.2.4/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/@hotwired%2fturbo/7.2.4/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/@hotwired%2fturbo/7.2.4/compatibility-slim/7.2.0)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/@hotwired%2fturbo/7.2.4/confidence-slim/7.2.0)](https://docs.renovatebot.com/merge-confidence/)
|
| [lit](https://lit.dev/) ([source](https://togithub.com/lit/lit)) |
[`2.3.1` -> `2.4.0`](https://renovatebot.com/diffs/npm/lit/2.3.1/2.4.0)
|
[![age](https://badges.renovateapi.com/packages/npm/lit/2.4.0/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/lit/2.4.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/lit/2.4.0/compatibility-slim/2.3.1)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/lit/2.4.0/confidence-slim/2.3.1)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>hotwired/turbo</summary>

### [`v7.2.4`](https://togithub.com/hotwired/turbo/releases/tag/v7.2.4)

[Compare
Source](https://togithub.com/hotwired/turbo/compare/v7.2.3...v7.2.4)

#### What's Changed

- Update history during same-page Visits by
[@&#8203;seanpdoyle](https://togithub.com/seanpdoyle) in
[https://github.com/hotwired/turbo/pull/763](https://togithub.com/hotwired/turbo/pull/763)
- Check last rendered location for same-page visits by
[@&#8203;kevinmcconnell](https://togithub.com/kevinmcconnell) in
[https://github.com/hotwired/turbo/pull/772](https://togithub.com/hotwired/turbo/pull/772)

**Full Changelog**:
hotwired/turbo@v7.2.2...v7.2.4

###
[`v7.2.3`](https://togithub.com/hotwired/turbo/compare/v7.2.2...v7.2.3)

[Compare
Source](https://togithub.com/hotwired/turbo/compare/v7.2.2...v7.2.3)

### [`v7.2.2`](https://togithub.com/hotwired/turbo/releases/tag/v7.2.2)

[Compare
Source](https://togithub.com/hotwired/turbo/compare/v7.2.1...v7.2.2)

#### What's Changed

- Fix double `before-fetch-request` dispatch during reload by
[@&#8203;seanpdoyle](https://togithub.com/seanpdoyle) in
[https://github.com/hotwired/turbo/pull/740](https://togithub.com/hotwired/turbo/pull/740)
- Ignore UJS `<a>` clicks and `<form>` submissions by
[@&#8203;seanpdoyle](https://togithub.com/seanpdoyle) in
[https://github.com/hotwired/turbo/pull/744](https://togithub.com/hotwired/turbo/pull/744)
- Cache PageSnapshot with original HTML from the frame's page by
[@&#8203;manuelpuyol](https://togithub.com/manuelpuyol) in
[https://github.com/hotwired/turbo/pull/746](https://togithub.com/hotwired/turbo/pull/746)
- Fix data-turbo-action was not respected on requestSubmit() event from
javascript by [@&#8203;seanpdoyle](https://togithub.com/seanpdoyle) in
[https://github.com/hotwired/turbo/pull/749](https://togithub.com/hotwired/turbo/pull/749)
- Reload page if failed form changes tracked content by
[@&#8203;kevinmcconnell](https://togithub.com/kevinmcconnell) in
[https://github.com/hotwired/turbo/pull/759](https://togithub.com/hotwired/turbo/pull/759)

(Note: 7.2.1 got pushed to npm as a copy of 7.2.0 as a mistake, hence
the extra jump in the tiny version).

**Full Changelog**:
hotwired/turbo@v7.2.0...v7.2.2

###
[`v7.2.1`](https://togithub.com/hotwired/turbo/compare/v7.2.0...v7.2.1)

[Compare
Source](https://togithub.com/hotwired/turbo/compare/v7.2.0...v7.2.1)

</details>

<details>
<summary>lit/lit</summary>

###
[`v2.4.0`](https://togithub.com/lit/lit/blob/HEAD/packages/lit/CHANGELOG.md#&#8203;240)

[Compare
Source](https://togithub.com/lit/lit/compare/847f4ba8570eb86d05be4378a3c954a941d2c063...lit@2.4.0)

##### Minor Changes

- [#&#8203;3318](https://togithub.com/lit/lit/pull/3318)
[`21313077`](https://togithub.com/lit/lit/commit/21313077669c19b3d631a50825b8a01dae1dd0d4)
- Adds an `isServer` variable export to `lit` and
`lit-html/is-server.js` which will be `true` in Node and `false` in the
browser. This can be used when authoring components to change behavior
based on whether or not the component is executing in an SSR context.

##### Patch Changes

- [#&#8203;3320](https://togithub.com/lit/lit/pull/3320)
[`305852d4`](https://togithub.com/lit/lit/commit/305852d4a4f51174301720985de98fdbf8674648)
- The `lit` package now specifies and "types" export condition allowing
TypeScript `moduleResolution` to be `nodenext`.

- Updated dependencies
\[[`21313077`](https://togithub.com/lit/lit/commit/21313077669c19b3d631a50825b8a01dae1dd0d4)]:
    -   lit-html@2.4.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 6am on monday" in timezone
Australia/Sydney, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/google/osv.dev).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC45LjEiLCJ1cGRhdGVkSW5WZXIiOiIzNC45LjEifQ==-->

Co-authored-by: Rex P <106129829+another-rex@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

frame.reload() triggers turbo:before-fetch-request callback twice
2 participants