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

Sveltekit aliased imports support #14952

Closed
madeleineostoja opened this issue May 17, 2021 · 22 comments
Closed

Sveltekit aliased imports support #14952

madeleineostoja opened this issue May 17, 2021 · 22 comments

Comments

@madeleineostoja
Copy link

madeleineostoja commented May 17, 2021

Describe the bug
The new @storybook/svelte setup breaks when you try and integrate with Sveltekit, because it relies on using aliased imports generated at runtime like $app/env.

I tried adding these aliases to webpack directly, eg:

webpackFinal: async (config) => {
    config.resolve.alias = {
      ...config.resolve?.alias,
      $app: path.resolve(__dirname, '../.svelte-kit/dev/runtime/app')
    };
    return config;
  }

But then I get parsing errors, I assume because it's an ES module and storybook needs to transpile it

File was processed with these loaders:
 * ./node_modules/babel-loader/lib/index.js
You may need an additional loader to handle the result of these loaders.
|  */
| 
> var browser = !import.meta.env.SSR;

System

System:
    OS: macOS 11.2.1
    CPU: (4) x64 Intel(R) Core(TM) i5-7360U CPU @ 2.30GHz
  Binaries:
    Node: 15.9.0 - /usr/local/bin/node
    Yarn: 1.22.4 - ~/.yarn/bin/yarn
    npm: 7.5.3 - /usr/local/bin/npm
  Browsers:
    Chrome: 90.0.4430.212
    Firefox: 74.0
    Safari: 14.0.3
  npmPackages:
    @storybook/addon-actions: ^6.2.9 => 6.2.9 
    @storybook/addon-essentials: ^6.2.9 => 6.2.9 
    @storybook/addon-links: ^6.2.9 => 6.2.9 
    @storybook/addon-svelte-csf: ^1.0.0 => 1.0.0 
    @storybook/svelte: ^6.2.9 => 6.2.9 

Additional context
I imagine using the upcoming pluggable builders with Vite (with ES build support, which sveltekit uses under the hood) might fix this issue, but I couldn't get the alpha storybook-vite-builder working either.

@madeleineostoja
Copy link
Author

FWIW also opened an issue for this in Svelte/kit because it sounds like it might need a solution upstream, unless there's something about webpack aliases and storybook's babel runtime I'm missing sveltejs/kit#1485

@shilman
Copy link
Member

shilman commented May 27, 2021

@j3rem1e any chance you can take a quick look?

@j3rem1e
Copy link
Contributor

j3rem1e commented May 27, 2021

vite/snowpack uses 'import.meta', which is not supported by webpack 4.

Sadly, webpack5 supports 'import.meta' but not 'import.meta.env' . You should probably mock '$app/env' (ie, aliasing $app/env to a file you own), or try to use the vite-builder.

svelte-kit is more and more dependent of vite.

@shilman
Copy link
Member

shilman commented May 28, 2021

@j3rem1e can we figure out how to get it running in @storybook/builder-vite

storybookjs/builder-vite#16

@madeleineostoja
Copy link
Author

ah that makes sense then. Mocking all the runtime imports of sveltekit ($app/env is just one) would probably be more work than its worth, especially if getting the vite builder running smoothly would fix it. I think as long as hardcoded aliases to the .svelte-kit files that are generated during dev work that's a reasonable gotcha — you'd have to have had run svelte-kit dev before storybook will work, I remember a similar limitation with Gatsby shimming back in the day. We can just document those aliases, there'd be 4 of them ($app/env, $app/navigation, $app/paths, $app/stores)

@shilman
Copy link
Member

shilman commented May 28, 2021

@madeleineostoja Ooh those aliases sound like a good workaround! I think we can get the vite builder working, but I suspect it will be awhile before that is officially supported, and in the meantime it would be good to have a solution for webpack. Any chance you can help add that?

@madeleineostoja
Copy link
Author

By the sounds of it webpack 5 won’t support the full import.meta.* that sveltekit uses though? The runtime aliases have that syntax, that was the original issue unfortunately. So it sounds like vite will be the only reasonable answer for now

@madeleineostoja
Copy link
Author

FYI seems the problem with the vite builder is addon-svelte-csf depending on webpack storybookjs/builder-vite#16 (comment)

@madeleineostoja
Copy link
Author

I can confirm that with @storybook/[email protected], [email protected], and @storybook/[email protected] this is now fixed 🎉

You can add a manual alias to Sveltekit's runtime modules in viteFinal and they work, with the caveat being that you have to have had run sveltekit dev previously.

module.exports = {
  core: { builder: 'storybook-builder-vite' },
  async viteFinal(config) {
    config.resolve.alias = {
      $app: path.resolve('./.svelte-kit/dev/runtime/app'),
    };

    return config;
  }
};

I think this is worth documenting somewhere, probably in the docs for @storybook/svelte, because sveltekit is the biggest use case for storybook + svelte, and it's broken out of the box currently. Hopefully the svelte team comes up with a more robust way to consume those runtime modules outside of sveltekit, but for now this workaround is at least fairly transparent, and I remember doing similar things with Gatsby and Storyboook so 🤷‍♀️

@shilman
Copy link
Member

shilman commented Jun 16, 2021

@jonniebigodes @j3rem1e can you two please figure out the best way to document this? it's weird because SvelteKit depends on vite community builder, which is not part of core. But we're going to have to figure this out and I don't want to lose more Svelte users.

@madeleineostoja
Copy link
Author

Also just in case you wanted to document the other option for the runtime aliases, this would be the path based on sveltekit build

path.resolve('./.svelte-kit/build/runtime/app')

If you used ^ you'd probably want to make your storybook run script

svelte-kit build && start-storybook

Might be a better default for documentation, doesn't have the gotcha of having to have had run sveltekit dev recently

@michaelwooley
Copy link

Not sure if this was ever documented. However, just got this working and thought I'd share the slight tweaks that have probably come up in last few months:

  • Needed to pin the dependency: [email protected]. Newer versions appear not to handle main.cjs well? Only looks for main.js.
  • Needed to add an additional configuration option for vite:
config.server.fsServe.strict = false;
config.server.fsServe.allow = ['.'];          // <~ new

For everything else, following along with what @madeleineostoja 's did here works well.

Nothing spectacular but it did take some time to work out the right set of tweaks! Oh well.

@shilman
Copy link
Member

shilman commented Sep 21, 2021

cc @eirslett @IanVS ☝️

@IanVS
Copy link
Member

IanVS commented Sep 22, 2021

Newer versions appear not to handle main.cjs well? Only looks for main.js.

Hmmm, that seems odd. Would you be willing to open an issue in https://github.com/eirslett/storybook-builder-vite with the details of what you are seeing, @michaelwooley?

@nielsvandermolen
Copy link

nielsvandermolen commented Jan 27, 2022

A way to work around the issue is to mock the functions yourself.

If the component uses import {goto} from $app/navigation

I created a .storybook/app/navigation.js file and exported the goto function:

export function goto() {
  console.log("hello storybook trying to call goto but we mocked it.");
}

Then we can link the $app alias to the .storybook app folder by adding this to the webpack4 config:

config.resolve.alias = {
...
  $app: path.resolve(__dirname, '../.storybook/app/')
...

Full webpack config:

  webpackFinal: async (config) => {
    config.module.rules.push({
      test: [/\.stories\.js$/, /index\.js$/],
      use: [require.resolve('@storybook/source-loader')],
      include: [path.resolve(__dirname, '../src')],
      enforce: 'pre'
    });
    config.resolve.alias = {
      ...config.resolve.alias,
      $lib: path.resolve(__dirname, '../src/lib'),
      $app: path.resolve(__dirname, '../.storybook/app/')
    };
    return config;
  },

At least this solution will allow you to run a storybook and create the components with webpack 4.

@benmccann
Copy link
Contributor

benmccann commented Jul 6, 2022

https://github.com/michaelwooley/storybook-experimental-vite demonstrates setting up Storybook with the new vite.config.js, which fixes $app/env

@benmccann
Copy link
Contributor

$app/env is now well supported by adding the SvelteKit Vite plugin in viteFinal. I've been looking at the other SvelteKit aliases and, as a novice Storybook user, I'm not sure what if anything we should do to support them. My first inclination is that if your components are well-structured you would not need to use any of these aliases. E.g. a generic button component wouldn't need to navigate, but could accept an action that would result in navigation.

Do folks have use cases for the other aliases besides $app/env that they can describe?

@madeleineostoja
Copy link
Author

madeleineostoja commented Jul 10, 2022

My first inclination is that if your components are well-structured you would not need to use any of these aliases. E.g. a generic button component wouldn't need to navigate, but could accept an action that would result in navigation.

I think that's a fairly broad and opinionated assumption. Taking your own example of a button that needs in-app navigatation, unless you're distributing that component as part of a consumable design system, making app navigation a seperate action on every instance of that button sounds a lot more like a workaround for these kind of issues than a good design pattern. It's like saying you should always pass react router's Link component as a prop rather than just using it in your app's components directly.

In any case $app/navigation is a trivial example, something like $app/stores is more problematic, where components can and absolutely should access global Sveltekit state directly, since that's the point of global state.

Tbh I've seen this kind of rationale a bunch of times from the Svelte team in different discussions and it irks me — "what you're all doing that svelte can't support right now is bad practice, do it our way". Idk just feels off, and very different from just flagging that something doesn't work for x technical reason and has y as a workaround/alternate pattern

@jamesjwarren
Copy link

jamesjwarren commented Sep 14, 2022

$app/env is now well supported by adding the SvelteKit Vite plugin in viteFinal.

The $app/env example seems to be broken again with the latest sveltekit (1.0.0-next.481)... the import appears to have been moved to $app/environment https://kit.svelte.dev/docs/modules#$app-environment. I would've expected the same alias configuration to work as outlined above:

// .storybook/main.js
module.exports = {
  //...
  async viteFinal(config, { configType }) {
    config.resolve.alias = {
      $app: path.resolve("./.svelte-kit/runtime/app"),
    };
    return config;
  },
};

But with this we get the following error, that the module can't be found:

Could not load .svelte-kit/runtime/app/environment (imported by src/components/_shared/Blog/Preview/Preview.svelte): 
ENOENT: no such file or directory, open '.svelte-kit/runtime/app/environment'

@shilman shilman removed the P2 label Oct 18, 2022
@IanVS
Copy link
Member

IanVS commented Jan 10, 2023

This should be supported in Storybook 7.0 using the sveltekit framework (@storybook/sveltekit), right @JReinhold?

@benmccann
Copy link
Contributor

I'd have guessed it's blocked by #20239?

@benmccann
Copy link
Contributor

Most aliases are now supported out-of-the-box with the latest Storybook 7. You can see a summary of what is supported and not supported here: https://github.com/storybookjs/storybook/tree/next/code/frameworks/sveltekit

There are a few that are not yet supported and make more sense to support as mocks. I've created a new issue to track that: #20999. PRs for it would be very welcome!

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