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

Core: Rerun loaders when args/globals change #16476

Merged
merged 2 commits into from
Oct 27, 2021

Conversation

tmeasday
Copy link
Member

Issue: #16260

What I did

  • Move renderToDOM warning to a more sensible place
  • Simplified render logic to always call loaders
  • Ensure we always abort the previous render
  • Track if we are doing the first actual render (because we aborted the earlier one) and if so force a remount.

How to test

  • Is this testable with Jest or Chromatic screenshots?

@nx-cloud
Copy link

nx-cloud bot commented Oct 26, 2021

Nx Cloud Report

CI ran the following commands for commit 32c4776. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Looking good 💯

@shilman shilman added this to the 6.4 PRs milestone Oct 26, 2021
@shilman shilman changed the title Rerun loaders when args/globals change Core: Rerun loaders when args/globals change Oct 26, 2021
@shilman shilman merged commit 2adb400 into next Oct 27, 2021
@shilman shilman deleted the 16260-rerun-loaders-on-rerender branch October 27, 2021 17:02
@pahan35
Copy link

pahan35 commented Oct 28, 2021

It looks like after this release, there are some bugs with the URL params.

E.g., I have URLs like http://localhost:6006/?path=/story/example-button--large&args=size:!undefined;primary:false

Recorded example

@tmeasday
Copy link
Member Author

Hey @pahan35 what version of SB is that there and what does the repo look like? Can you post the code somewhere?

@pahan35
Copy link

pahan35 commented Oct 28, 2021

It is a usual repro project from npx sb repro with react like

Environment Info:

System:
OS: macOS 11.6
CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
Binaries:
Node: 14.17.6 - /usr/local/opt/node@14/bin/node
Yarn: 3.0.2 - /usr/local/bin/yarn
npm: 6.14.15 - /usr/local/opt/node@14/bin/npm
Browsers:
Chrome: 95.0.4638.54
Safari: 15.0
npmPackages:
@storybook/addon-actions: ^6.4.0-beta.21 => 6.4.0-beta.21
@storybook/addon-docs: ^6.4.0-beta.21 => 6.4.0-beta.21
@storybook/addon-essentials: ^6.4.0-beta.21 => 6.4.0-beta.21
@storybook/addon-links: ^6.4.0-beta.21 => 6.4.0-beta.21
@storybook/react: ^6.4.0-beta.21 => 6.4.0-beta.21

@pahan35
Copy link

pahan35 commented Oct 29, 2021

@tmeasday mentioned bug looks fixed in 6.4.0-beta.22.

@tmeasday
Copy link
Member Author

Thanks @pahan35

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

Successfully merging this pull request may close these issues.

None yet

3 participants