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

Full page load on navigation since 5.1.0-alpha.37 #6765

Closed
lewisl9029 opened this issue May 11, 2019 · 5 comments
Closed

Full page load on navigation since 5.1.0-alpha.37 #6765

lewisl9029 opened this issue May 11, 2019 · 5 comments
Assignees
Milestone

Comments

@lewisl9029
Copy link

Describe the bug

In previous releases, navigating to a new story using the sidebar seems to have only resulted in a js navigation event inside the preview iframe.

Unfortunately, since 5.1.0-alpha.37 (specifically this PR: #6688), navigating to a new story has started to trigger a full page load inside the iframe, which can add quite a bit of friction for large projects that have several MBs of JS to load, parse and run.

To Reproduce

Steps to reproduce the behavior:

  1. Go to https://monorepo-git-5925-fix-kind-deep-linking.storybook.now.sh/examples/official-storybook/?path=/story/addons-backgrounds--story-1
  2. Click on "story 2" in the sidebar
  3. Iframe content blinks from the full page load. Can also check network logs to verify resources within the iframe being reloaded. (see gifs below)

Expected behavior

Content should not blink, and network logs should show no new resources getting loaded.

This can be verified with the official storybook instance following the same steps (or simply see gifs below): https://storybooks-official.netlify.com/?path=/story/addons-backgrounds--story-1

Screenshots

Behavior after #6688:
May-10-2019 18-29-16

Expected:
May-10-2019 18-29-28

System:

  • OS: MacOS
  • Device: Macbook Pro 2018
  • Browser: firefox, chrome, safari
  • Framework: react
  • Version: 5.1.0-alpha.37

Additional Context

I imagine there's probably some existing mechanism in storybook that turns route change events in the top level frame into js navigation events inside the preview iframe, otherwise the route inside the iframe would have never changed prior to #6688.

Maybe we can fix deep linking by tapping into that functionality instead of re-rendering the iframe?

@shilman shilman added this to the 5.1.0 milestone May 11, 2019
@shilman
Copy link
Member

shilman commented May 11, 2019

@tmeasday Mind taking a look at this?

@tmeasday
Copy link
Member

I wondered about this; I should have probably looked closer.

Thanks for the detailed report @lewisl9029.

The reason that #6688 was needed was due to the iframe not yet being initialized when the first event was sent over the channel; thus the usual mechanism not working.

I guess we need to
(a) Explicitly ensure the src of the iframe never changes. Previous to #6688 this happened but it was really just an accident AFAICT.
(b) figure out some mechanism for buffering the initial events that are sent over the channel before the iframe intializes (or some other mechanism to achieve the same effect).

Any ideas @ndelangen / others?

@ndelangen
Copy link
Member

(b) figure out some mechanism for buffering the initial events that are sent over the channel before the iframe intializes (or some other mechanism to achieve the same effect).

Sounds like the proper plan tbh

@stale
Copy link

stale bot commented Jun 4, 2019

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Jun 4, 2019
@ndelangen
Copy link
Member

Should be fixed in 5.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants