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

[6.0] Cannot add an args enhancer to the store after a story has been added. #327

Closed
kyle-ssg opened this issue Feb 12, 2022 · 45 comments
Closed

Comments

@kyle-ssg
Copy link

kyle-ssg commented Feb 12, 2022

Describe the bug
Whenever I edit a story I see the following error.

 ERROR  Error: Cannot add an args enhancer to the store after a story has been added., js engine: hermes

To Reproduce
Steps to reproduce the behavior:

  1. Edit a story
  2. Error displays

Expected behavior
I can edit/add stories without errors

Screenshots

Code snippets
If applicable, add code samples to help explain your problem.

System:
Please paste the results of npx -p @storybook/cli@next sb info here.

  System:
    OS: macOS 11.2.2
    CPU: (12) x64 AMD Ryzen 5 5600X 6-Core Processor             
  Binaries:
    Node: 16.13.2 - ~/.nvm/versions/node/v16.13.2/bin/node
    npm: 8.1.2 - ~/.nvm/versions/node/v16.13.2/bin/npm
  Browsers:
    Chrome: 98.0.4758.80
    Firefox: 94.0.2
    Safari: 14.0.3
  npmPackages:
    @storybook/addon-actions: ^6.3.1 => 6.3.13 
    @storybook/addon-links: ^6.3.1 => 6.4.18 
    @storybook/addon-ondevice-actions: ^6.0.1-alpha.7 => 6.0.1-alpha.7 
    @storybook/addon-ondevice-backgrounds: ^6.0.1-alpha.7 => 6.0.1-alpha.7 
    @storybook/addon-ondevice-controls: ^6.0.1-alpha.7 => 6.0.1-alpha.7 
    @storybook/addon-ondevice-notes: ^6.0.1-alpha.7 => 6.0.1-alpha.7 
    @storybook/addons: ^6.3.1 => 6.4.18 
    @storybook/react-native: ^6.0.1-alpha.7 => 6.0.1-alpha.7 
    @storybook/react-native-server: ^6.0.0-alpha.0 => 6.0.0-alpha.0 

Additional context
I'm following the exact same code as in the native example in this repo with a couple of basic stories e.g.

import { ComponentStory } from '@storybook/react-native'
import Loader from 'components/base/Loader'
export default {
  title: 'Loader',
  component: Loader,
}

export const Default: ComponentStory<typeof Loader> = (
  args: typeof Default.args,
) => (
  <>
    <Loader {...args} />
  </>
)

Default.args = {}

@kyle-ssg
Copy link
Author

As a note, patching these errors out and replacing with a "return;" I notice no issues with storybook.

@dannyhw
Copy link
Member

dannyhw commented Feb 16, 2022

Hey @kyle-ssg thanks for reporting this issue. I think it's probably an incompatibility issue with 6.4.

 @storybook/addon-actions: ^6.3.1 => 6.3.13 
    @storybook/addon-links: ^6.3.1 => 6.4.18 
@storybook/addons: ^6.3.1 => 6.4.18 

your addons version is resolving to 6.4, currently 6.4 is not yet supported please use ~6.3 instead. I believe it might resolve your issue.

After you try it let me know and if it didn't work then I'm happy to debug further.

@ainsleyrutterford
Copy link

I'm getting the same error when editing stories. I'm using 6.3 for addons:

System:
    OS: macOS 11.1
    CPU: (16) x64 Intel(R) Core(TM) i7-10700K CPU @ 3.80GHz
  Binaries:
    Node: 14.16.1 - ~/.nvm/versions/node/v14.16.1/bin/node
    Yarn: 1.22.10 - ~/.nvm/versions/node/v14.16.1/bin/yarn
    npm: 6.14.13 - ~/.nvm/versions/node/v14.16.1/bin/npm
  Browsers:
    Chrome: 98.0.4758.102
    Firefox: 97.0.1
    Safari: 14.0.2
  npmPackages:
    @storybook/addon-actions: 6.3.1 => 6.3.1 
    @storybook/addon-controls: 6.3.1 => 6.3.1 
    @storybook/addon-ondevice-actions: ^6.0.1-beta.1 => 6.0.1-beta.3 
    @storybook/addon-ondevice-backgrounds: ^6.0.1-beta.1 => 6.0.1-beta.3 
    @storybook/addon-ondevice-controls: ^6.0.1-beta.1 => 6.0.1-beta.3 
    @storybook/addon-ondevice-notes: ^6.0.1-beta.1 => 6.0.1-beta.3 
    @storybook/react-native: ^6.0.1-beta.1 => 6.0.1-beta.3 

@dannyhw
Copy link
Member

dannyhw commented Feb 21, 2022

@ainsleyrutterford could you provide a public repository with a reproduction of the problem so I can debug it?

@ainsleyrutterford
Copy link

@ainsleyrutterford could you provide a public repository with a reproduction of the problem so I can debug it?

Ah, the project I'm working on is private. I'll see if I can start a new project and reproduce the error.

@levino
Copy link

levino commented Apr 6, 2022

Same issue here.

@dannyhw
Copy link
Member

dannyhw commented Apr 6, 2022

@levino a reproduction would be really great if possible 🙏

@levino
Copy link

levino commented Apr 7, 2022

The amount of work to make a reproduction example is quite high. I will keep it in mind. The errors occur when one changes some code somewhere and then the app gets a refresh of the new code. I guess the configuration is run again, even though it should not have been.

@dannyhw
Copy link
Member

dannyhw commented Apr 7, 2022

The problem is that without a reproduction its a lot of work for me to maybe succeed at reproducing an issue. I will try soon though, just need to find the time.

@patrikniebur
Copy link

I have reproduced the issue here https://github.com/patrikniebur/storybook-react-native-error
It seems to happen when I render the storybook UI inside of the component like so:
https://github.com/patrikniebur/storybook-react-native-error/blob/9eed3f924e755ff796e1f57f92b17f5f4167f0dc/App.js#L5-L7

But if I export the storybook UI as a default from the same file the issue disappears. My specific reason to try to render the StorybookUI inside of the component and not as the root component is embedding it into the app as one of the hidden routes.

@dannyhw
Copy link
Member

dannyhw commented Apr 18, 2022

@patrikniebur thanks for the repro 🙏 I'll take a look tonight and see what I can find

@dannyhw
Copy link
Member

dannyhw commented Apr 18, 2022

@patrikniebur be careful with your dependencies

    "@storybook/addon-actions": "^6.3.13",
    "@storybook/addon-controls": "^6.3.13",

these should be either a fixed version at 6.3.13 or ~6.3.13 since 6.4 isn't yet supported.

@dannyhw
Copy link
Member

dannyhw commented Apr 19, 2022

With the reproduction I'm able to reproduce this. I'm working on finding the cause, thanks again for the reproduction @patrikniebur!

@dannyhw
Copy link
Member

dannyhw commented Apr 19, 2022

like @patrikniebur mentioned it seems like you can avoid this by exporting the storybookUI directly like

import StorybookUIRoot from './.storybook/Storybook';
export default StorybookUIRoot;

instead of

import StorybookUIRoot from './.storybook/Storybook';
function App() {
  return StorybookUIRoot();
}
export default App;

So for now you can use that as a workaround.

Still investigating.

@dannyhw
Copy link
Member

dannyhw commented Apr 19, 2022

I'm thinking that I'll release a temporary fix for this whilst I figure out the root cause. It shouldn't require user changes though. Will update once its live.

@samgalaxycard
Copy link

I am getting the same error while adding more than 1 stories file inside components

@samgalaxycard
Copy link

Also this line gets automatically added in my storybook.requires.js file which throws this error:
error: Error: Unable to resolve module [object Object]/register from /home/gct/app/.storybook/storybook.requires.js: [object Object]/register could not be found within the project or in these directories:
node_modules
../node_modules

@dannyhw
Copy link
Member

dannyhw commented Apr 24, 2022

@samgalaxycard seems like you might be missing a dependency or some configuration.

I have the temporary fix ready for this, just need to find time to release it (maybe tomorrow).

Seems like there is a deeper issue though. A lot of changes are coming soon that would make this easier to solve.

dannyhw pushed a commit that referenced this issue Apr 25, 2022
@dannyhw
Copy link
Member

dannyhw commented Apr 25, 2022

@kyle-ssg @samgalaxycard @patrikniebur I've published a temporary fix in 6.0.1-beta.6, could you try updating and running the sb-rn-get-stories command to update your requires file.

@patrikniebur
Copy link

patrikniebur commented Apr 25, 2022

Thanks @dannyhw but unfortunately I'm still getting the same error. I've upgraded the version in my test case and I'm still getting the same error

@dannyhw
Copy link
Member

dannyhw commented Apr 25, 2022

@patrikniebur you need to run the script to update the requires file

I tested on the same example and it works.

@patrikniebur
Copy link

Right, sorry I forgot about that part! Thanks for the fix, that's going to help a lot at my work!
Another thing I've noticed is that hot reloading does not work when I edit the story file, not the component file itself. I guess this was happening before but was obfuscated by the error. Should I create new issue for this to keep track of it?
I have added replication into the test case repository linked above.

@dannyhw
Copy link
Member

dannyhw commented Apr 26, 2022

@patrikniebur by hot reload do you mean that it always full refreshes instead of doing a fast refresh? That specifically is a known bug with an issue and PR already #11 #30

The only issue is the pr with a fix seems to break storiesOf which is already deprecated however I'm not sure I want to commit to fully removing it. It's a complicated issue but its on the list of things to resolve.

@patrikniebur
Copy link

I mean no refresh at all. But I assume that's because of the same error that is being hidden now. Anyway editing component works fine with hot reload so I'm happy. Thanks again!

@dannyhw
Copy link
Member

dannyhw commented Apr 27, 2022

@patrikniebur interesting, I'm not sure why that would happen. I'll definitely have to revisit hot reloading.

@tyom
Copy link

tyom commented May 3, 2022

I'm pretty sure it used to refresh correctly when editing a component before the fix. It was blowing up when editing a story. Now it swallows the error, so it appears that the underlying issue is still there.

@dannyhw
Copy link
Member

dannyhw commented May 3, 2022

@tyom used to when? Catching the error is a workaround but it wouldn't be the thing that breaks reloading. Also if you see prior comments if you change the way you export the storybook ui then it should not even error and maybe would hot reload. Need to test this.

@tyom
Copy link

tyom commented May 3, 2022

@dannyhw I believe it used to refresh correctly when editing components prior to the latest fix in #351. However, when editing stories the refresh failed with the originally reported error. Now it says that the view has been refreshed without any changes. Editing the component works a treat though. It would be great if it refreshed correctly when editing a story as well.

In addition, I have a global decorator and when editing the story (where refresh is reported but ignored) I'm getting a duplicate decorator warning. I wonder if it's related.

image

@dannyhw
Copy link
Member

dannyhw commented May 3, 2022

@tyom yes I believe the code in preview is getting executed on every hot reload which shouldn't be the case unless that file changes. I've got some ideas on how to potentially fix that but I'm not an expert on hot reloading so it might take a lot of trial and error 😅 .

If anyone can help that would be much appreciated, otherwise I'll explore some potential solutions as soon as I have an evening or weekend free.

@levino
Copy link

levino commented Jul 1, 2022

Just as an update: This is still very broken for me / us. Basically we have to reload the whole app every time we change some code. This is not the case for the real app. There all updates get streamed and applied.

We also load the storybook on demand like so:

import React, { useState, useEffect } from 'react'
import { DevSettings } from 'react-native'
import { loadString, saveString } from '~storage'
/**
 * Toggle Storybook mode, in __DEV__ mode only.
 *
 * In non-__DEV__ mode, or when Storybook isn't toggled on,
 * renders its children.
 *
 * The mode flag is persisted in async storage, which means it
 * persists across reloads/restarts - this is handy when developing
 * new components in Storybook.
 */
export function ToggleStorybook(props: any) {
    const [showStorybook, setShowStorybook] = useState(false)
    const [StorybookUIRoot, setStorybookUIRoot] = useState<React.FC>(
        null as unknown as React.FC
    )

    useEffect(() => {
        if (__DEV__ && DevSettings) {
            // Load the setting from storage if it's there
            loadString('devStorybook').then((storedSetting) => {
                // Set the initial value
                setShowStorybook(storedSetting === 'on')

                // Add our toggle command to the menu
                DevSettings.addMenuItem('Toggle Storybook', () => {
                    setShowStorybook((show) => {
                        // On toggle, flip the current value
                        show = !show

                        // Write it back to storage
                        saveString('devStorybook', show ? 'on' : 'off')

                        // Return it to change the local state
                        return show
                    })
                })
                // Add these lines line to re-enable storybook
                // Load the storybook UI once
                setStorybookUIRoot(
                    // eslint-disable-next-line @typescript-eslint/no-var-requires
                    () => require('../.storybook/Storybook').StorybookUIRoot
                )
            })
        }
    }, [])

    if (showStorybook) {
        return StorybookUIRoot ? <StorybookUIRoot /> : null
    } else {
        return props.children
    }
}

@dannyhw
Copy link
Member

dannyhw commented Jul 1, 2022

@levino hey sorry that you're still experiencing issues. I can see that you have an interesting way of loading storybook.

What is the reason to use a state variable like this? Why not just require inline like:

const StorybookUIRoot = require('../.storybook/Storybook').StorybookUIRoot
return <StorybookUIRoot />

Though I guess you got this from the ignite boilerplate right?

Anyway, I can have a look at this again and I'll get back to you.

@levino
Copy link

levino commented Jul 1, 2022

This enables us to just toggle the storybook on and off via the dev menu in react native. Found this somewhere on the internet. If you want I can try to find the source. It works pretty well but for the update functionality.

@dannyhw
Copy link
Member

dannyhw commented Jul 3, 2022

@levino ok but have you tried removing this stuff too see if it might be the cause of the hot reloading not working?

If you directly export storybookui for example does your storybook reload properly?

@levino
Copy link

levino commented Jul 6, 2022

Just a ping back: I am not able to reproduce reliably. It happens currently with either import - sometimes :( I will further investigate and come back to this.

@dannyhw
Copy link
Member

dannyhw commented Oct 27, 2022

I think this will be resolved atleast partly in #384 which I will hopefully be merging soon.

@levino
Copy link

levino commented Oct 28, 2022

Our issue was resolved by not using the storiesOf syntax but the CSF. storiesOf usage results in broken reload.

@dannyhw
Copy link
Member

dannyhw commented Oct 28, 2022

@levino fast refresh is fixed in #384 for both storiesOf and CSF. You can try it in 6.0.1-canary.1 if you want. Though soon I will merge it into the beta.

also storiesOf is deprecated for the v6 beta so I still recommend to use csf

@dannyhw
Copy link
Member

dannyhw commented Oct 22, 2023

This should be resolved in v6.5.6 already and in v7 this will work quite differently so I'm closing this. Let me know if you want to reopen.

@dannyhw dannyhw closed this as completed Oct 22, 2023
@arekkubaczkowski
Copy link

@dannyhw I am using v7 and unfortunately still having this issue. Specifically I am getting Storybook is not ready yet error. I tried both inline require and esm import. I Would be grateful for any kind of help

@dannyhw
Copy link
Member

dannyhw commented Mar 11, 2024

@arekkubaczkowski please specify which version your are on because this should be resolved. Make sure you are on the latest version for all related packages.

@arekkubaczkowski
Copy link

arekkubaczkowski commented Mar 11, 2024

@dannyhw I am on the latest version 7.6.15

@arekkubaczkowski
Copy link

I've just noticed that there were two releases lately and one of them fixes hot reloading 😅 let me check

@dannyhw
Copy link
Member

dannyhw commented Mar 11, 2024

@arekkubaczkowski the latest version is 7.6.17 and it has a hot reloading fix

@dannyhw
Copy link
Member

dannyhw commented Mar 11, 2024

make sure to regenerate the requires file as the fix is in the generated file.

@arekkubaczkowski
Copy link

works like a charm! thanks a lot!

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

8 participants