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]: Config.base not getting appended correctly #20138

Closed
tomasbillborn opened this issue Dec 7, 2022 · 27 comments
Closed

[Bug]: Config.base not getting appended correctly #20138

tomasbillborn opened this issue Dec 7, 2022 · 27 comments

Comments

@tomasbillborn
Copy link

Describe the bug

Second http-request of runtime-mjs => sb-preview is not getting config.base appended correctly in Storybook 7 when deployed to github pages resulting in a 404

To Reproduce

I am building storybook in a git workflow...

npx cross-env BASE_URL=https://xxxxxx.github.io/DesignSystem/ npm run build-storybook

^build-story in package.json
"build-storybook": "storybook build"

in .storybook/main.js

async viteFinal(config) {
    config.base = process.env.BASE_URL || config.base;
    return config;
}

First http-request
/DesignSystem/sb-preview/runtime.mjs

Second http-request
/sb-preview/runtime.mjs (404)

System

Environment Info:

  System:
    OS: Windows 10 10.0.19045
    CPU: (8) x64 Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz
  Binaries:
    Node: 18.12.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.4 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 8.19.2 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 107.0.5304.123
    Edge: Spartan (44.19041.1266.0), Chromium (108.0.1462.42)
  npmPackages:
    @storybook/addon-actions: ^7.0.0-alpha.61 => 7.0.0-alpha.61
    @storybook/addon-docs: ^7.0.0-alpha.61 => 7.0.0-alpha.61
    @storybook/addon-essentials: ^7.0.0-alpha.61 => 7.0.0-alpha.61
    @storybook/addon-interactions: ^7.0.0-alpha.61 => 7.0.0-alpha.61
    @storybook/addon-links: ^7.0.0-alpha.61 => 7.0.0-alpha.61
    @storybook/addon-postcss: ^2.0.0 => 2.0.0
    @storybook/react: ^7.0.0-alpha.61 => 7.0.0-alpha.61
    @storybook/react-vite: ^7.0.0-alpha.61 => 7.0.0-alpha.61
    @storybook/testing-library: 0.0.14-next.0 => 0.0.14-next.0

Additional context

No response

@tomasbillborn
Copy link
Author

@IanVS you asked me to tag you here after creating the bug

@IanVS
Copy link
Member

IanVS commented Dec 7, 2022

When you mention the first and second requests, are these in a single page load, or when you refresh the page?

What happens when you don't set the config.base with BASE_URL, does that make any difference?

This is failing in github pages, right? Does it also fail if you use npx http-server <path/to/storybook/output>?

@tomasbillborn
Copy link
Author

storybook-7-alpha-61

When I load the page this is the http-requests. I am not reloading anything and when I do reload its the same problem. As you can see the first request has the correct config.base and gets the requests correctly the second at the bottom doesnt

When we used Storybook 6 I had to set the config.base for it to understand the storybook static site was hosted on our github-page

/DesignSystem/

I can try to remove the config.base and get back to you soon.

@tomasbillborn
Copy link
Author

tomasbillborn commented Dec 7, 2022

What happens when you don't set the config.base with BASE_URL, does that make any difference?
It does not seem to make a difference when I removed it, config.base that is.

This is failing in github pages, right? Does it also fail if you use npx http-server <path/to/storybook/output>?
Yes in github pages.
No it works as it should when I build it local and serve it from http-server

The difference here is I have no /DesignSystem/ as a part of all http-requests when run from http-server

@IanVS

@IanVS
Copy link
Member

IanVS commented Dec 7, 2022

@ndelangen we'll have to look into this, whether the way we are serving /sb-preview/runtime.mjs is broken in github pages.

@tomasbillborn
Copy link
Author

@IanVS @shilman any progress on solving this?

@shilman
Copy link
Member

shilman commented Dec 13, 2022

@tomasbillborn Not yet. Working through high priority bugs identified by the beta. This is on the list!

@marvinroger
Copy link

I have exactly the same problem, in an nginx.

The "second" failing request comes from this import in iframe.js:

import "/sb-preview/runtime.mjs";

Maybe this should be

import "./sb-preview/runtime.mjs";

Instead?

@marvinroger
Copy link

One more bit of information: I added a nginx rewrite rule to fix the issue specifically for /sb-preview/runtime.mjs and ended up having another failing import: /sb-preview/chunk-I2FDEMEK.mjs. So the runtime.mjs is not the only issue here 😉

@ndelangen
Copy link
Member

@marvinroger I think the nginx rewrite-rule doesn't change the url request in the browser, so I think this PR might actually fix the problem: #20232

@marvinroger
Copy link

marvinroger commented Dec 13, 2022

No, it's a server-side workaround on my end, clearly not a solution.

On first look, it seems like your PR is fixing the runtime.mjs issue, but as stated above it's not the only problem unfortunately.

@tomasbillborn
Copy link
Author

This problem only happens when there is a config.base change, never locally when one is not present. So most likely when you are in the iframe context this config.base is not appended.

@ndelangen
Copy link
Member

I'm fairly confident my PR will fix the issue.

Screenshot 2022-12-13 at 18 30 23

runtime.mjs makes a relative-URL request, so if the runtime.mjs resolves from the correct path, the request it makes should work as well.

@ndelangen ndelangen self-assigned this Dec 14, 2022
@shilman
Copy link
Member

shilman commented Dec 14, 2022

Ooh-la-la!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-beta.7 containing PR #20232 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Dec 14, 2022
@marvinroger
Copy link

I can confirm the issue is fixed on my side with the latest beta, thanks a lot for your reactivity!

@IanVS
Copy link
Member

IanVS commented Dec 14, 2022

Thanks for reporting back!

@ndelangen
Copy link
Member

Awesome!

@tomasbillborn
Copy link
Author

Now sb-preview works, thank you for that!

But for some reason
I am getting 404 on

_getPrototype.632da1a4.js
_commonjsHelpers.712cc82f.js
_baseToString.f30149a5.js

all files starting with an underscore and I am not getting this locally. I double checked Actions in github and the files are built

Failed to fetch dynamically imported module: https://xxx.github.io/DesignSystem/assets/preview.367c954f.js
TypeError: Failed to fetch dynamically imported module: https://xxx.github.io/DesignSystem/assets/preview.367c954f.js

Any clue why this could happen? @IanVS @ndelangen ?

@ndelangen
Copy link
Member

@tomasbillborn I do not, but those are files generated by the vite builder, I can't be 100% sure without investigating fully, but from the looks of things that's just vite's output, and I'd have no idea why github isn't properly hosting those.

@tomasbillborn
Copy link
Author

By default, Jekyll doesn't build files or folders that:

are located in a folder called /node_modules or /vendor
start with _, ., or #
end with ~
are excluded by the exclude setting in your configuration file
If you want Jekyll to process any of these files, you can use the include setting in your configuration file.

Is there a special reason these files must be named with the convention "_"? @ndelangen @IanVS @shilman

I am guessing I am not the only one that is going to have to this problem and it wasnt that easy to understand.

@ndelangen
Copy link
Member

@tomasbillborn Is there anything in your vite config that might be causing this?

@tomasbillborn
Copy link
Author

No the problem is files that start with "_" are by default seen as files that should be not be copied

By default, Jekyll doesn't build files
...
start with _, ., or #

@ndelangen

@marvinroger
Copy link

I don't think it's in the storybook scope to handle the specificities of the different hostings.

@tomasbillborn you can create a .nojekyll in the root of the repo / branch to bypass Jekyll (see https://github.blog/2009-12-29-bypassing-jekyll-on-github-pages/), then all files will be considered by GitHub Pages.

This could, however, be a tip in the storybook docs. 😉

@ndelangen
Copy link
Member

You're asking me why does the filename start with a _ and I have to ask the same question back, because we don't make that happen anywhere AFAIK.

I understand the leading _ is a problem for you and why. But I'm saying there's a reason for that, I don't think that reason is explicitly storybook.

We do not configure vite to prefix things with _, nor do we do that elsewhere..

@tomasbillborn
Copy link
Author

tomasbillborn commented Dec 15, 2022

I never said I didnt get it to work. I added the .nojekyll file, but when you build with

gh-pages (npm)

for some reason you have to add an undocumented flag (tschaub/gh-pages#315)

-t true

for it to actually add things correct such as a touch .nojekyll file into the branch I am deploying to.

What I said was there are most likely more people then me that uses github pages to host storybook. So I never told you to change it, I asked if there was no particular reason for the name convention maybe you could change it to make life easier for others who are going to run into this. I appreciate your fast response and all the help you given @ndelangen

@IanVS
Copy link
Member

IanVS commented Dec 15, 2022

@tomasbillborn thanks for bringing it up. Storybook itself is not intentionally creating these files. I'm not positive where they're coming from, but they might be generated by babel or vite or some other tool in the chain. It's unfortunate that GitHub pages don't play nicely with them by default, but I'm not sure there is anything we could do to name them differently, unfortunately.

@tomasbillborn
Copy link
Author

Well maybe what @marvinroger said is the best way forward. Put it in the documentation if possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

5 participants