Use 'will render' false for action 'replace' after GET redirect by blrB · Pull Request #516 · hotwired/turbo

blrB · 2022-01-20T11:03:16Z

Todo list:

fix issue
pass previous tests
add tests

Closes #515

marphi · 2022-01-26T19:25:47Z

In my project I've noticed the same problem with redirects. Looks like this PR fix it. @blrB thx a lot!

blrB · 2022-04-30T21:54:10Z

I fixed eslint errors in my code. All other errors in files not from my branch.

➜  turbo git:(double_rendering_for_get_redirect) yarn lint                                               
yarn run v1.22.17
$ eslint . --ext .ts

/Users/babkou/tmp/turbo/src/core/drive/page_renderer.ts
  15:48  error  Insert `,`  prettier/prettier
  21:43  error  Insert `,`  prettier/prettier

/Users/babkou/tmp/turbo/src/core/native/browser_adapter.ts
  11:14  error  Replace `[key:·string]:·any` with `·[key:·string]:·any·`  prettier/prettier
  57:23  error  Insert `,`                                                prettier/prettier
  58:12  error  Insert `,`                                                prettier/prettier

/Users/babkou/tmp/turbo/src/core/session.ts
  131:33  error  Insert `,`  prettier/prettier

✖ 6 problems (6 errors, 0 warnings)
  6 errors and 0 warnings potentially fixable with the `--fix` option.

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
➜  turbo git:(double_rendering_for_get_redirect

blrB · 2022-04-30T22:25:45Z

All eslint errors from other code was fixed too. (Beacuse GitHub CI can't be valide with any errors)

➜  turbo git:(double_rendering_for_get_redirect) yarn lint
yarn run v1.22.17
$ eslint . --ext .ts
✨  Done in 6.64s.
➜  turbo git:(double_rendering_for_get_redirect)

timpwbaker · 2022-07-14T13:57:52Z

Thanks! This fixed this issue for me too!

When redirecting to a page that contains elements marked with `[data-turbo-cache="false"]`, those elements are removed _before_ the initial render, instead of _after_ the render and _before_ the page is cached. This behavior seems to have stemmed from [hotwired#516][], which was shipped in response to [hotwired#515][]. As an alternative to the `willRender: false` option passed to `this.adapter.visitProposedToLocation` in `Visit.followRedirect`, the implementation can instead [rely on the presence of the `turbo-frame[complete]`][comment] to guard against double fetching. To guard against regressions, this commit adds coverage for the unwanted behavior by redirecting from `navigation.html` to `cache_observer.html`, and asserting the presence of a `[data-turbo-cache="false"]` element that resembles and application's Flash messaging. [hotwired#515]: hotwired#515 [hotwired#516]: hotwired#516 [comment]: hotwired#515 (comment) [hotwired#487]: hotwired#487

When redirecting to a page that contains elements marked with `[data-turbo-cache="false"]`, those elements are removed _before_ the initial render, instead of _after_ the render and _before_ the page is cached. This behavior seems to have stemmed from [#516][], which was shipped in response to [#515][]. As an alternative to the `willRender: false` option passed to `this.adapter.visitProposedToLocation` in `Visit.followRedirect`, the implementation can instead [rely on the presence of the `turbo-frame[complete]`][comment] to guard against double fetching. To guard against regressions, this commit adds coverage for the unwanted behavior by redirecting from `navigation.html` to `cache_observer.html`, and asserting the presence of a `[data-turbo-cache="false"]` element that resembles and application's Flash messaging. [#515]: #515 [#516]: #516 [comment]: #515 (comment) [#487]: #487

When redirecting to a page that contains elements marked with `[data-turbo-cache="false"]`, those elements are removed _before_ the initial render, instead of _after_ the render and _before_ the page is cached. This behavior seems to have stemmed from [hotwired/turbo#516][], which was shipped in response to [hotwired/turbo#515][]. As an alternative to the `willRender: false` option passed to `this.adapter.visitProposedToLocation` in `Visit.followRedirect`, the implementation can instead [rely on the presence of the `turbo-frame[complete]`][comment] to guard against double fetching. To guard against regressions, this commit adds coverage for the unwanted behavior by redirecting from `navigation.html` to `cache_observer.html`, and asserting the presence of a `[data-turbo-cache="false"]` element that resembles and application's Flash messaging. [hotwired/turbo#515]: hotwired/turbo#515 [hotwired/turbo#516]: hotwired/turbo#516 [comment]: hotwired/turbo#515 (comment) [hotwired/turbo#487]: hotwired/turbo#487

fix(visit): will render false for action 'replace' after redirect

6ba1181

blrB changed the title ~~Use will render false for action 'replace' after GET redirect~~ Draft: Use will render false for action 'replace' after GET redirect Jan 20, 2022

blrB changed the title ~~Draft: Use will render false for action 'replace' after GET redirect~~ Draft: Use 'will render' false for action 'replace' after GET redirect Jan 20, 2022

blrB mentioned this pull request Jan 20, 2022

Double fetch requests for eager-loaded frames #515

Closed

blrB force-pushed the double_rendering_for_get_redirect branch from 2afffb9 to c891dcd Compare January 20, 2022 22:39

feat(eager frame): add test for check fetch eager frame count

ba5a8a3

blrB force-pushed the double_rendering_for_get_redirect branch from c891dcd to ba5a8a3 Compare January 20, 2022 22:41

blrB changed the title ~~Draft: Use 'will render' false for action 'replace' after GET redirect~~ Use 'will render' false for action 'replace' after GET redirect Jan 20, 2022

blrB mentioned this pull request Feb 12, 2022

Turbo frame loads twice when it is in the response of a redirected request #526

Closed

dfritsch mentioned this pull request Feb 15, 2022

When following a redirect, the resulting HTML is rendered to the page twice #537

Closed

feliperaul mentioned this pull request Apr 3, 2022

Fix turbo:load firing twice after a redirect and avoid double render #563

Merged

dhh and others added 2 commits April 30, 2022 14:03

Merge branch 'main' into double_rendering_for_get_redirect

5df3d7d

ref(tests): fix eslint errors

03b0f35

blrB force-pushed the double_rendering_for_get_redirect branch from 13e0cde to 03b0f35 Compare April 30, 2022 21:43

ref(test): fix eslint Insert errors

a1224e9

ref(eslint): fix all errors from eslint in branch

47d4e66

Merge branch 'hotwired:main' into double_rendering_for_get_redirect

17145d7

dhh merged commit 88bf29a into hotwired:main Jun 19, 2022

seanpdoyle mentioned this pull request Aug 9, 2022

Bugfix: Redirects and [data-turbo-cache=false] #674

Merged

blrB mentioned this pull request Oct 26, 2022

Fix double fetch requests (render) for eager-loaded frames #779

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use 'will render' false for action 'replace' after GET redirect#516

Use 'will render' false for action 'replace' after GET redirect#516
dhh merged 7 commits intohotwired:mainfrom
blrB:double_rendering_for_get_redirect

blrB commented Jan 20, 2022 •

edited

Loading

Uh oh!

marphi commented Jan 26, 2022 •

edited

Loading

Uh oh!

blrB commented Apr 30, 2022 •

edited

Loading

Uh oh!

blrB commented Apr 30, 2022

Uh oh!

timpwbaker commented Jul 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Conversation

blrB commented Jan 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marphi commented Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blrB commented Apr 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blrB commented Apr 30, 2022

Uh oh!

timpwbaker commented Jul 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

blrB commented Jan 20, 2022 •

edited

Loading

marphi commented Jan 26, 2022 •

edited

Loading

blrB commented Apr 30, 2022 •

edited

Loading