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

Relative URLs to assets no longer work in CSS now that CSS no longer gets inlined #4645

Closed
Vinnl opened this issue Oct 30, 2018 · 39 comments · Fixed by storybookjs/presets#119
Labels
cra Prioritize create-react-app compatibility question / support react typescript

Comments

@Vinnl
Copy link

Vinnl commented Oct 30, 2018

Describe the bug
I just upgraded to Storybook 4, and I think this issue is caused by that (it required a lot of other moving parts as well).

Basically, in my React component, I import './Widget.css';. Then in Widget.css, I reference a background image located in the same folder as such: background-image: url(orcid_16x16.png);.

With Storybook 3, this just worked even using build-storybook: the image URL was updated to static/media/orcid_16x16.2dddb203.png, and since the CSS was inline, this path was correct.

Now with Storybook 4, however, the URL is still updated to that value, but because it is now located in a CSS file that itself located in static/css, the browser is now trying to find the image at static/css/static/media/, and hence cannot find it.

To Reproduce
Steps to reproduce the behavior:

  1. Visit my built Storybook v3
  2. Notice that there is a green icon to the left-hand side of Alice Doe
  3. Now visit that same page in Storybook 4
  4. Notice that the image is missing

Expected behavior
The image to also be present in Storybook 4 - presumably by making sure that URL imports are rewritten to be relative to the CSS file they are in.

Screenshots

In Storybook 3:
image

In Storybook 4:
image

System:

  • OS: Ubuntu Linux 18.04
  • Device: Dell XPS 13 9360
  • Browser: Firefox
  • Framework: React
  • Addons: [if relevant]
  • Version: 4.0.0
@igor-dv
Copy link
Member

igor-dv commented Oct 31, 2018

Do you have any custom webpack.config?

@Vinnl
Copy link
Author

Vinnl commented Oct 31, 2018

@igor-dv Ah, yes, sorry - I'm using TypeScript.

The config in the case of the working images:

const path = require("path");

module.exports = (baseConfig, env, defaultConfig) => {
  defaultConfig.module.rules.push({
    test: /\.(ts|tsx)$/,
    include: [
      path.resolve(__dirname, "../src"),
      path.resolve(__dirname, "../stories"),
    ],
    loader: require.resolve("ts-loader"),
    options: { configFile: '../tsconfig.json' }
  });
  defaultConfig.resolve.extensions.push(".ts", ".tsx");

  return defaultConfig;
};

and the config in the case where images do not load:

const path = require("path");

module.exports = (baseConfig, env, defaultConfig) => {
  defaultConfig.module.rules.push({
    test: /\.(ts|tsx)$/,
    include: [
      path.resolve(__dirname, "../src"),
      path.resolve(__dirname, "../stories"),
    ],
    loader: require.resolve('awesome-typescript-loader'),
    options: { configFileName: path.resolve(__dirname, './tsconfig.json') }
  });
  defaultConfig.resolve.extensions.push(".ts", ".tsx");

  return defaultConfig;
};

The difference between the two is that the loader changed from ts-loader to awesome-typescript-loader.

@igor-dv
Copy link
Member

igor-dv commented Oct 31, 2018

So the difference in the awesome-typescript-loader vs ts-loader, right ? 🤔

@Vinnl
Copy link
Author

Vinnl commented Oct 31, 2018

Yes, as far as the Webpack config is concerned. Though i don't think they do anything with image imports, especially when done from CSS?

@igor-dv
Copy link
Member

igor-dv commented Oct 31, 2018

Did you try to use SB v4 with the ts-loader (to eliminate this problem)? Nothing is changed in png loading in a new release...

@Vinnl
Copy link
Author

Vinnl commented Oct 31, 2018

Well, I wanted to, but unfortunately I couldn't get it to work - I ran into this error message:

Module build failed (from ./node_modules/ts-loader/index.js):
Error: TypeScript emitted no output for <..>/stories/widget.tsx.
at makeSourceMapAndFinish (<..>/node_modules/ts-loader/dist/index.js:78:15) 
at successLoader (<..>/node_modules/ts-loader/dist/index.js:68:9)
at Object.loader (<..>/node_modules/ts-loader/dist/index.js:22:12)

Since the official docs also use awesome-typescript-loader, I figured I'd try it with that, and that worked. (I used to use ts-loader because my app used that as well, but that is no longer the case.)

@igor-dv
Copy link
Member

igor-dv commented Oct 31, 2018

BTW, how does your tsconfig look like?
(I am asking different questions because idk what is the problem 🤷‍♂️ )

Also, if you could share a public reproduction, that'd be great.

@Vinnl
Copy link
Author

Vinnl commented Oct 31, 2018

Haha, I was afraid so :P

The tsconfig.json is not that special:

{
  "compilerOptions": {
    "target": "es5",
    "allowJs": true,
    "skipLibCheck": true,
    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true,
    "strict": true,
    "forceConsistentCasingInFileNames": true,
    "module": "esnext",
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "isolatedModules": true,
    "noEmit": true,
    "jsx": "react",
    "lib": ["esnext", "dom"]
  },
  "include": [
    "src"
  ],
  "exclude": [
    "src/setupProxy.js"
  ]
}

I don't have a minimal reproduction unfortunately, but the project this occurs on luckily is open source: with Storybook 3, with Storybook 4. (Note that there are several projects in that repo - the one with Storybook is located in /client.)

@igor-dv
Copy link
Member

igor-dv commented Oct 31, 2018

You can exclude stories from tsconfig and create another one for storybook (tsconfig.sb.json) that will extend your regular tsconfig. In this new tsconfig you can remove the "noEmit": true, (it could be a root of your initial ts-loader problem). Excluding stories from the tsconfig could boost your apps building time.

@Vinnl
Copy link
Author

Vinnl commented Oct 31, 2018

@igor-dv Hehe, that's what I was doing already - I just mashed them together here for easier display :)

Setting noEmit: false did solve ts-loader not working. Unfortunately, images are still not loaded :(

@igor-dv
Copy link
Member

igor-dv commented Oct 31, 2018

Looks like I need to clone the project 😴

@Vinnl
Copy link
Author

Vinnl commented Oct 31, 2018

Ah don't bother, that's far too much effort! I'll probably be diving into this again tomorrow; who knows, I might get some fresh insights :)

@igor-dv
Copy link
Member

igor-dv commented Oct 31, 2018

I see that you've migrated to react-scripts as well. We have a feature that integrates styling from the CRA2.

CC: @chadfawcett , could it be somehow related to this?

@chadfawcett
Copy link
Member

I believe this is because you're switching to CRA. I believe the file paths need to be relative. So change orcid_16x16.png to ./orcid_16x16.png.

https://facebook.github.io/create-react-app/docs/adding-images-fonts-and-files
facebook/create-react-app#4574 (comment)

@chadfawcett
Copy link
Member

Although, I wasn't actually able to replicate the issue. I cloned the repo, checked out the branch, and ran yarn storybook. This may be because I'm on MacOS, and it does a bit of magic when it comes to resolving file paths.

screen shot 2018-10-31 at 10 50 29 am

@Vinnl
Copy link
Author

Vinnl commented Oct 31, 2018

@chadfawcett Actually, it used to be like that - I just changed it to see whether that fixes it. I'll try changing it back, but I think orcid_16x16.png is relative as well - especially considering that this URL is changed in the built version, just not to the correct path.

Note that it does work locally (i.e. unbuilt) for me as well, because the CSS is still inline at that point, so the relative import path still works. It just doesn't work in the built version because the CSS file is then in a subfolder, and imports are resolved by the browser relative to that.

Edit: Just tried it with a ./ import - the images still won't load, unnfortunately.

Thanks for your help both, though, you're really going above and beyond :)

@chadfawcett
Copy link
Member

Sorry @Vinnl, I missed the fact that this was specific to built version.

Looks like it's a bug in the storybook cra webpack preset. I'll try to get a PR in tonight.

@Vinnl
Copy link
Author

Vinnl commented Nov 1, 2018

That'd be fantastic @chadfawcett. If you have any pointers I'd be happy to take a shot at it myself, if need be :)

@Vinnl
Copy link
Author

Vinnl commented Nov 1, 2018

Some more observations: CRA successfully turns it into an absolute URL, as can be seen here. However, it knows how to do so because the developer configures at what location the app will be deployed. Given that that location is different for the built Storybook application, I'm not sure how to deal with that other than providing a similar configuration option for Storybook itself.

Though perhaps an alternative is to always inline images as data URLs, given that data use is probably (?) less of an issue for Storybooks. Or, alternatively, not to extract the CSS into separate files at all, like in Storybook 3.

Anyway, I'm not sure if that is desirable from a Storybook point of view, so that's probably best for you to judge.

@chadfawcett
Copy link
Member

@Vinnl Apologies that this took longer than expected. I got caught up with some testing issues.

Take a look at #4695. Would you be able to clone my branch and test it with your repo? I've already done so, but it would be good if you could confirm the fix resolves your issue. Here's how to include the local build in your project https://github.com/storybooks/storybook/blob/master/CONTRIBUTING.md#working-with-your-own-app

@Vinnl
Copy link
Author

Vinnl commented Nov 2, 2018

No problem @chadfawcett, I'm already indebted to you for even trying to fix it, let alone actually doing so :)

Anyway, I saw the PR and was already trying it out - I've just confirmed that it does, indeed, fix the issue :)

@Vinnl
Copy link
Author

Vinnl commented Nov 6, 2018

Just upgraded to 4.0.4, and it works flawlessly. Thanks!

@shilman shilman added the cra Prioritize create-react-app compatibility label Nov 22, 2018
@tulsidaskhatri
Copy link

I am getting exactly same issue with
"@storybook/react": "^5.3.7",

css is trying to look for a font file in
static/css/static/media/myfont.woff2

but the font is at following url:
static/media/myfont.woff2

@ppiyush13
Copy link

I am also facing same issue

@VinceVachon
Copy link

Same "5.3.9",

@shilman
Copy link
Member

shilman commented Feb 14, 2020

@mrmckeb did something change in the CRA preset that might affect this?

@mrmckeb
Copy link
Member

mrmckeb commented Feb 19, 2020

This looks like a conflict between Storybook and CRA's configurations, which is odd. I can take a look at this.

Has anyone got a reproduction they can share with me? Thanks!

@YDundie
Copy link

YDundie commented Mar 5, 2020

Same for the version 5.3.13

@YDundie
Copy link

YDundie commented Mar 5, 2020

Hello.
Successfully solved it by adding this to my .storybook/main.js

const path = require('path');

const publicPath = process.env.DEV ? "http://localhost:9009/" :"https://your-web-address.com/"

module.exports = {
	stories: ['../src/**/*.stories.js'],
	addons: [
		'@storybook/preset-create-react-app',
		'@storybook/addon-actions',
		'@storybook/addon-links',
		'@storybook/addon-knobs'],

	webpackFinal: async (config, { configType }) => {
	
		config.output.publicPath=publicPath

		return config;
	},
};


Hopefully it could help you.

@cherouvim
Copy link

cherouvim commented Mar 23, 2020

Has anyone got a reproduction they can share with me? Thanks!

Hello. This is the most minimal repro I could come up with. Steps:

  1. Confirm that the woff font file is loading via the network tab:
npm install
npm run storybook
  1. Confirm that font file is not loading (404s due to static/css/static/media):
npm run build-storybook
# view generated index.html

storybook-repro-4645.zip

@maniax89
Copy link
Contributor

in my project - i was able to remedy this by adding the "homepage": "." to my package json -> https://create-react-app.dev/docs/deployment/#serving-the-same-build-from-different-paths

not sure if this helps anyone^

seems like the relevant lines in CRA look something like this (https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/config/webpack.config.js):

  // common function to get style loaders
  const getStyleLoaders = (cssOptions, preProcessor) => {
    const loaders = [
      isEnvDevelopment && require.resolve('style-loader'),
      isEnvProduction && {
        loader: MiniCssExtractPlugin.loader,
        // css is located in `static/css`, use '../../' to locate index.html folder
        // in production `paths.publicUrlOrPath` can be a relative path
        options: paths.publicUrlOrPath.startsWith('.')
          ? { publicPath: '../../' }
          : {},
      },

and public path is set below:

      // webpack uses `publicPath` to determine where the app is being served from.
      // It requires a trailing slash, or the file assets will get an incorrect path.
      // We inferred the "public path" (such as / or /my-project) from homepage.
      publicPath: paths.publicUrlOrPath,

@shilman
Copy link
Member

shilman commented Mar 23, 2020

@mrmckeb any chance you can look at @cherouvim 's repro?

@mrmckeb
Copy link
Member

mrmckeb commented Mar 24, 2020

@cherouvim, I downloaded your project and ran it - and I see "Rumba" loading as expected.

image

You also have a webpack.config.js file which is not needed, as CRA inserts the necessary config.

I then migrated the project to use the new preset for CRA, and everything was still working correctly.

Can you please provide some more info? Thanks!

@mrmckeb
Copy link
Member

mrmckeb commented Mar 24, 2020

@YDundie, I'd like to find the root cause of this and fix it in the preset. Are you able to share an example of what isn't working for you?

Even just code snippets here.

@cherouvim
Copy link

@mrmckeb it does work on npm run storybook, but does it work on npm run build-storybook?

@mrmckeb
Copy link
Member

mrmckeb commented Mar 24, 2020

Thanks @cherouvim, I didn't understand that it was build only. Found and hopefully fixed. Thanks for the solution @maniax89!

@ZeeshanTamboli
Copy link

@maniax89's solution to add "homepage": "." in package.json worked for me. Thanks!

@rodoabad
Copy link

rodoabad commented Jan 28, 2021

This is still happening on non-CRA apps.

In CSS.

// foo.module.scss
background: url('./logo.svg') 30px center no-repeat $white;

After Storybook.

static/media/logo.f6b1d730.svg

@kneupper
Copy link

kneupper commented Mar 16, 2022

For anybody still having this issue down the road on a non-CRA app, please reference this SO thread

The key of this is in specifying { url: false } in the CSS Module rule of your webpack config. Here is the specific configuration I used for my CSS Module rule:

config.module.rules.push({
    test: /\.module\.css$/,
    use: [
        require.resolve('style-loader'),
        {
            loader: require.resolve('css-loader'),
            options: {
                modules: {...}, // project-specific module naming conventions, config, etc.
                url: false,
                importLoaders: 1,
            },
        },
    ],
}) 

Hope this can save some time for anybody else!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cra Prioritize create-react-app compatibility question / support react typescript
Projects
None yet
Development

Successfully merging a pull request may close this issue.