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] dist/preset.js doesn't exist #6

Closed
oliversturm opened this issue Mar 15, 2021 · 10 comments · Fixed by #7
Closed

[Bug] dist/preset.js doesn't exist #6

oliversturm opened this issue Mar 15, 2021 · 10 comments · Fixed by #7
Labels
bug Something isn't working

Comments

@oliversturm
Copy link

Describe the bug

The main item in package.json refers to dist/preset.js - but this does not exist, the correct name is dist/preset/index.js.

I noticed this in my Svelte/Snowpack based project, since Snowpack was complaining about the missing file during it's dependency building step. I worked around by manually changing the entry to dist/preset/index.js (or just dist/preset, this works as well) and then Snowpack is happy.

@oliversturm oliversturm added the bug Something isn't working label Mar 15, 2021
@j3rem1e
Copy link
Contributor

j3rem1e commented Mar 15, 2021

@phated this come from the default addon-kit.
Is it an issue from the template, or something wrong in the build configured here ?

@shilman
Copy link
Member

shilman commented Mar 16, 2021

Probably a bug in the addon-kit cc @winkerVSbecks

@oliversturm
Copy link
Author

oliversturm commented Mar 16, 2021

Follow-up: I have found a rather strange issue. At least it seems strange to me, maybe I haven't investigated enough.

  • My project uses Snowpack as a dev environment.
  • This was working fine, and then I added Storybook. I set things up so Storybook worked correctly.
  • However, Snowpack had issues because it pulls in addon-svelte-csf as part of its dependency build step. In encounters the invalid main entry in package.json - as I reported - and runs into trouble.
  • I fixed package.json for this addon - again as reported - and now Snowpack was happy.
  • However, now Storybook didn't work anymore! During startup, I saw an error that began like this:
ERROR in ./node_modules/@storybook/addon-svelte-csf/dist/parser/svelte-stories-loader.js
Module not found: Error: Can't resolve 'fs' in '/home/sturm/git/myproject/node_modules/@storybook/addon-svelte-csf/dist/parser'
 @ ./node_modules/@storybook/addon-svelte-csf/dist/parser/svelte-stories-loader.js 11:10-23
 @ ./node_modules/@storybook/addon-svelte-csf/dist/preset/index.js
 @ ./src/stories/OliButton.stories.svelte
 @ ./src sync ^\.(?:(?:^|\/|(?:(?:(?!(?:^|\/)\.).)*?)\/)(?!\.)(?=.)[^/]*?\.stories\.svelte)$
 @ ./.storybook/generated-stories-entry.js
...

I suspect that I can configure Snowpack to ignore the .stories.js files - there's really no reason why they should be included.

However, I thought I should mention the issue I'm seeing. I don't understand why the Storybook startup suddenly has this trouble about resolving fs when I change the main entry to refer to a valid file. Please let me know if I can help with additional information. I'll try to find a workaround now by configuring Snowpack to exclude the unnecessary items.

@j3rem1e
Copy link
Contributor

j3rem1e commented Mar 16, 2021

can you try with a field browser: dist/index.js in your package.json ?

Your story use an import from @storybook/addon-svelte-csf which resolve to the preset.js with your fix. preset can only be used in the backend, not in the browser

@oliversturm
Copy link
Author

Hi @j3rem1e ,

can you try with a field browser: dist/index.js in your package.json ?

I assume you mean in node_modules/@storybook/addon-svelte-csf/package.json? Okay - I added that field there, and this pacifies Snowpack. Good idea.

However, I still don't understand what the point is of referring to an invalid (non-existent) file in main, and why all the extra breakage occurs when I change it to refer to something that actually exists.

Anyway - with the browser field in place, I would not have noticed the issue in the first place. Meanwhile I have also managed to exclude the story files from Snowpack, which is the preferable solution to me anyway. So the question of the main field is now a mere curiosity to me. Thanks for your help!

@winkerVSbecks
Copy link

@oliversturm, dist/index.js should be available. But, for some reason the published package doesn't include it.

@j3rem1e Did you perhaps forget to run build before publishing it?

@oliversturm
Copy link
Author

@winkerVSbecks Yes, dist/index.js is available. But main refers to dist/preset.js, which does not exist.

@j3rem1e
Copy link
Contributor

j3rem1e commented Mar 16, 2021

@oliversturm when in webpack (and probably in snowpack then) you do "import 'xxx'", it checks the package.json do know what file it should load. if nothing is specified, it returns the root index.js, otherwise, in a browser, it checks the "browser" property then the "main" property, and in the server, it checks the "node" property, then the "main" property.

main should be a valid file, it's a bug currently to point to dist/preset/js instead of dist/preset, but you need the browser property in order to, in the browser, resolve @storybook/addon-svelte-csf to the Svelte Story components. If it work, I can do a PR to fix that, but I don't know how to reproduce it with snowpack.

@winkerVSbecks I didn't publish anything, I don't think I have the right to publish something to the storybook scope :)
but the bug is main referencing dist/preset.js which doesn't exist here.

@winkerVSbecks
Copy link

@j3rem1e right, thanks for clarifying. Yea, that indeed is a bug from the addon kit. I'll update it to use "main": "dist/preset" instead

@oliversturm
Copy link
Author

main should be a valid file, it's a bug currently to point to dist/preset/js instead of dist/preset

Agreed, that's what I found. But when I change it to dist/preset, I end up with errors about the missing fs module when I start storybook. I suspect you should be able to reproduce this error if you fix your own package.json main field.

I think setting the browser field is a good idea to solve the Snowpack issue - at least then nobody will run into trouble if they use Snowpack without excluding storybook stories explicitly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants