storybookjs / addon-svelte-csf

[Incubation] CSF using Svelte components.
MIT License
98 stars 29 forks source link

[Bug] When calling the fn "superForm" from "sveltekit-superforms/client" we get error "Cannot destructure property 'story' of 'storyObject' as it is undefined." #170

Open alexbjorlig opened 5 months ago

alexbjorlig commented 5 months ago

Describe the bug

I'm trying to write a story, and using sveltekit-superforms.

When calling the function superForm from import { superForm } from 'sveltekit-superforms/client'; I get the error:

TypeError: Cannot destructure property 'story' of 'storyObject' as it is undefined.
    at normalizeStory (http://localhost:6006/sb-preview/runtime.js:38:260)
    at http://localhost:6006/sb-preview/runtime.js:41:400
    at Array.forEach (<anonymous>)
    at processCSFFile (http://localhost:6006/sb-preview/runtime.js:41:354)
    at StoryStore.memoizerific [as processCSFFileWithCache] (http://localhost:6006/sb-preview/runtime.js:1:4751)
    at http://localhost:6006/sb-preview/runtime.js:47:8515
    at async StoryStore.loadStory (http://localhost:6006/sb-preview/runtime.js:47:9920)
    at async http://localhost:6006/sb-preview/runtime.js:81:9005
    at async StoryRender.runPhase (http://localhost:6006/sb-preview/runtime.js:81:8766)
    at async StoryRender.prepare (http://localhost:6006/sb-preview/runtime.js:81:8924)

Steps to reproduce the behavior

  1. Go to this repo, clone, and npm install
  2. Open the Button.stories.svelte in storybook
  3. See error

Expected behavior

No error.

Screenshots and/or logs

CleanShot 2024-01-30 at 19 41 02@2x

Environment

Storybook Environment Info:

  System:
    OS: macOS 14.3
    CPU: (8) arm64 Apple M2
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.17.1 - ~/.nvm/versions/node/v18.17.1/bin/node
    npm: 10.3.0 - ~/.nvm/versions/node/v18.17.1/bin/npm <----- active
  Browsers:
    Chrome: 121.0.6167.85
    Safari: 17.3
  npmPackages:
    @storybook/addon-essentials: ^7.6.11 => 7.6.11 
    @storybook/addon-interactions: ^7.6.11 => 7.6.11 
    @storybook/addon-links: ^7.6.11 => 7.6.11 
    @storybook/addon-svelte-csf: ^4.1.0 => 4.1.0 
    @storybook/blocks: ^7.6.11 => 7.6.11 
    @storybook/svelte: ^7.6.11 => 7.6.11 
    @storybook/sveltekit: ^7.6.11 => 7.6.11 
    @storybook/test: ^7.6.11 => 7.6.11 
    eslint-plugin-storybook: ^0.6.15 => 0.6.15 
    storybook: ^7.6.11 => 7.6.11

Additional context

I'm unsure if we are strictly speaking about a bug in Addon-svelte-csf, or if this has something to do with Superforms.

JReinhold commented 5 months ago

I'm not entirely sure what's going on, but I suspect superform is crashing, causing the whole story creation to fail, so the error we get is actually a result of the component crashing. In the console I see the following error first:

Error extracting stories TypeError: Invalid value used as weak map key TypeError: Invalid value used as weak map key
    at WeakMap.set (<anonymous>)
    at superForm (sveltekit-superforms_client.js?v=45b27c1d:2356:15)
    at instance (Button.stories.svelte?t=1706737241955:162:15)
    at init (chunk-PLRMHIPC.js?v=45b27c1d:2190:23)
    at new Button_stories (Button.stories.svelte?t=1706737241955:190:3)
    at createProxiedComponent (svelte-hooks.js?v=45b27c1d:341:9)
    at new ProxyComponent (proxy.js?v=45b27c1d:242:7)
    at new Proxy<Button.stories> (proxy.js?v=45b27c1d:349:11)
    at construct_svelte_component_dev (chunk-PLRMHIPC.js?v=45b27c1d:2634:22)
    at create_fragment (RegisterContext.svelte?v=45b27c1d:37:21)

That trace leads to this (compiled) source, (un-compiled source here):

  const _currentPage = get_store_value(page2);
  if (((_b = options.warnings) == null ? void 0 : _b.duplicateId) !== false) {
    if (!formIds.has(_currentPage)) {
      formIds.set(_currentPage, /* @__PURE__ */ new Set([_initialFormId]));

What's happening is that superform tries to do get(page), but the (mocked) page store is undefined, which then afterwards isn't a valid key for a WeakMap, causing the crash.

This comment hints that a page mock is always needed, however when using this addon the first render is always with an undefined page store, and then with a defined one afterwards. The following code in a X.stories.svelte file:

  import { get } from 'svelte/store';
  import { page } from '$app/stores';
  const pageValue = get(page);
  console.log('LOG: from store', { pageValue })

Will lead to these logs in the console:

image

I'm assuming this happens because we set the store values in a decorator, and the first render of Svelte CSF stories happens before the story function runs, thus without the decorators.

@paoloricciuti I'm wondering if we can initiate the mock SvelteKit stores with a default value at creation time to more closely match a SvelteKit experience? Is that even possible? If so, we should probably try to match the SvelteKit Page type, something like:

const emptyURL = new URL('');

const initialPageValue = {
  url: emptyURL,
  params: {},
  route: {
    id: null
  },
  status: 200,
  error: null,
  data: {},
  state: {},
  form: undefined
}
paoloricciuti commented 5 months ago

I'm not entirely sure what's going on, but I suspect superform is crashing, causing the whole story creation to fail, so the error we get is actually a result of the component crashing. In the console I see the following error first:

Error extracting stories TypeError: Invalid value used as weak map key TypeError: Invalid value used as weak map key
    at WeakMap.set (<anonymous>)
    at superForm (sveltekit-superforms_client.js?v=45b27c1d:2356:15)
    at instance (Button.stories.svelte?t=1706737241955:162:15)
    at init (chunk-PLRMHIPC.js?v=45b27c1d:2190:23)
    at new Button_stories (Button.stories.svelte?t=1706737241955:190:3)
    at createProxiedComponent (svelte-hooks.js?v=45b27c1d:341:9)
    at new ProxyComponent (proxy.js?v=45b27c1d:242:7)
    at new Proxy<Button.stories> (proxy.js?v=45b27c1d:349:11)
    at construct_svelte_component_dev (chunk-PLRMHIPC.js?v=45b27c1d:2634:22)
    at create_fragment (RegisterContext.svelte?v=45b27c1d:37:21)

That trace leads to this (compiled) source, (un-compiled source here):

  const _currentPage = get_store_value(page2);
  if (((_b = options.warnings) == null ? void 0 : _b.duplicateId) !== false) {
    if (!formIds.has(_currentPage)) {
      formIds.set(_currentPage, /* @__PURE__ */ new Set([_initialFormId]));

What's happening is that superform tries to do get(page), but the (mocked) page store is undefined, which then afterwards isn't a valid key for a WeakMap, causing the crash.

This comment hints that a page mock is always needed, however when using this addon the first render is always with an undefined page store, and then with a defined one afterwards. The following code in a X.stories.svelte file:

  import { get } from 'svelte/store';
  import { page } from '$app/stores';
  const pageValue = get(page);
  console.log('LOG: from store', { pageValue })

Will lead to these logs in the console:

image

I'm assuming this happens because we set the store values in a decorator, and the first render of Svelte CSF stories happens before the story function runs, thus without the decorators.

@paoloricciuti I'm wondering if we can initiate the mock SvelteKit stores with a default value at creation time to more closely match a SvelteKit experience? Is that even possible? If so, we should probably try to match the SvelteKit Page type, something like:

const emptyURL = new URL('');

const initialPageValue = {
  url: emptyURL,
  params: {},
  route: {
    id: null
  },
  status: 200,
  error: null,
  data: {},
  state: {},
  form: undefined
}

I'll have to take a look but I guess the problem is more in the fact that this addon mounts the svelte component to collect the stories. And that is actually run before decorators run.

In theory we could return a default value from the store if the context is not defined to at least solve this kind of errors. In this specific case I'm pretty sure we could "reimplement" the decorator in the addon itself to provide the same values.

I'll try to explore those solutions a bit

JReinhold commented 5 months ago

... I guess the problem is more in the fact that this addon mounts the svelte component to collect the stories...

Yes I agree this isn't ideal, I hope that we change that behavior during The Great Migration™, but it might not be feasable.

In theory we could return a default value from the store if the context is not defined to at least solve this kind of errors. In this specific case I'm pretty sure we could "reimplement" the decorator in the addon itself to provide the same values.

I'm just thinking a default value that resembles a barebone SvelteKit page would be better DX overall, and not necessarily just to solve this pre-decorator problem, but it might cause gotchas I'm aware of.

paoloricciuti commented 5 months ago

... I guess the problem is more in the fact that this addon mounts the svelte component to collect the stories...

Yes I agree this isn't ideal, I hope that we change that behavior during The Great Migration™, but it might not be feasable.

In theory we could return a default value from the store if the context is not defined to at least solve this kind of errors. In this specific case I'm pretty sure we could "reimplement" the decorator in the addon itself to provide the same values.

I'm just thinking a default value that resembles a barebone SvelteKit page would be better DX overall, and not necessarily just to solve this pre-decorator problem, but it might cause gotchas I'm aware of.

I think the headless mount is pretty genius actually maybe it's fixable with a custom AST transform but I agree, having a default that is closer to the sveltekit default can be beneficial

alexbjorlig commented 5 months ago

Hi guys, just wanted to say that your commitment to improving the Svelte/Sveltekit/Storybook experience is 10/10 🚀 Hope you find a good solution for this, and if I can help test/debug something just let me know

ciscoheat commented 5 months ago

Same here, it looks like the problem lies in the store mocking, but let me know if you need some Superforms debugging help.

alexbjorlig commented 4 months ago

@ciscoheat could we potentially "workaround" this issue for now, by adding another if storybook case?

ciscoheat commented 4 months ago

Sure, I'll add a fix for the next release.

ciscoheat commented 4 months ago

Added now in 2.8.1, which will make the above test repo work when upgraded, except for the submit functionality, which can be fixed when https://github.com/storybookjs/storybook/pull/26338 is merged.