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

[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 this issue Jan 30, 2024 · 9 comments
Labels
bug Something isn't working legacy Related to the legacy version(s)

Comments

@alexbjorlig
Copy link

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.

@alexbjorlig alexbjorlig added the bug Something isn't working label Jan 30, 2024
@alexbjorlig alexbjorlig changed the title [Bug] When calling the fn "superForm" from "sveltekit-superforms/client" we get error "annot destructure property 'story' of 'storyObject' as it is undefined." [Bug] When calling the fn "superForm" from "sveltekit-superforms/client" we get error "Cannot destructure property 'story' of 'storyObject' as it is undefined." Jan 31, 2024
@JReinhold
Copy link
Collaborator

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
Copy link
Contributor

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
Copy link
Collaborator

... 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
Copy link
Contributor

... 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
Copy link
Author

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
Copy link

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

@alexbjorlig
Copy link
Author

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

@ciscoheat
Copy link

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

@ciscoheat
Copy link

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 storybookjs/storybook#26338 is merged.

@xeho91 xeho91 added the legacy Related to the legacy version(s) label Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working legacy Related to the legacy version(s)
Projects
None yet
Development

No branches or pull requests

5 participants