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

[@next/storybook] - incorrectly imported css throws an inaccurate error #15195

Closed
jescalan opened this issue Jul 15, 2020 · 8 comments
Closed
Assignees
Labels
good first issue Easy to fix issues, good for newcomers please verify canary Please verify the issue with the latest canary branch.
Milestone

Comments

@jescalan
Copy link
Contributor

jescalan commented Jul 15, 2020

Bug report

Describe the bug

When a css file is imported in the wrong way, rather than displaying an error that describes the problem, the error can't resolve 'error-loader' is displayed, with no stack trace.

To Reproduce

  1. I made this repo for use with duplication
  2. Add a normal .css file and import it within one of the stories (this is not allowed, since normal css imports are only possible in _app.js)
  3. Observe the incorrect error message

Expected behavior

I would expect that the error message mirror that which you receive when using next.js normally - something along the lines of "global css imports are not allowed within pages, please use a css module or import within _app.js"

System information

  • OS: macOS
  • Version of Next.js: 9.4.4
  • Version of Node.js: 12.13.0
@jescalan
Copy link
Contributor Author

I realized that my reproduction steps here are inaccurate, will adjust and update shortly!

@jescalan
Copy link
Contributor Author

Updated with a custom reproduction repo!

@jnv
Copy link

jnv commented Aug 21, 2020

I ran into the same error, I guess there's a reason why @next/plugin-storybook is marked as experimental with the disclaimer:

This package is still very experimental and should not be used at this point

Anyway the reason why this happens is that Next's Webpack rules use internal error-loader which is nested under oneOf rule for (S)CSS loading. I have tried to relink the loader using require.resolve similarly how next-babel-loader is handled in the plugin, but it doesn't really work outside of context of Next app, I got this error when attempting to load SCSS file from Storybook's preview.js:

Global CSS cannot be imported from files other than your Custom . Please move all global CSS imports to src/pages/_app.tsx. Or convert the import to Component-Level CSS (CSS Modules).

Long story short, I have attempted to prune Next's S/CSS loading rules to work with Storybook but in the end I still didn't manage to get them to work – the best I got was having undefined for all imported CSS modules. If you would like to try it yourself, below is my customized, work-in-progress preset.js from the original plugin. I put it into .storybook/next-storybook.js and require webpackFinal into Storybook's main.js file. Note that this doesn't work.

// Based on https://github.com/vercel/next.js/blob/canary/packages/next-plugin-storybook/preset.js

const {PHASE_PRODUCTION_BUILD} = require('next/constants')
const {findPagesDir} = require('next/dist/lib/find-pages-dir')
const loadConfig = require('next/dist/next-server/server/config').default
const getWebpackConfig = require('next/dist/build/webpack-config').default

const CWD = process.cwd()

async function webpackFinal(config) {
  const pagesDir = findPagesDir(CWD)
  const nextConfig = await loadConfig(PHASE_PRODUCTION_BUILD, CWD)
  const nextWebpackConfig = await getWebpackConfig(CWD, {
    pagesDir,
    entrypoints: {},
    isServer: false,
    isProduction: false,
    target: 'server',
    config: nextConfig,
    buildId: 'storybook',
  })

  config.plugins = [...config.plugins, ...nextWebpackConfig.plugins]

  config.resolve = {
    ...config.resolve,
    ...nextWebpackConfig.resolve,
  }

  config.module.rules = [
    ...config.module.rules.filter((rule) => {
      // the rules we're filtering use RegExp for the test
      if (!rule.test instanceof RegExp) return true
      // use Next.js' built-in CSS
      if (rule.test.test('hello.css')) {
        return false
      }
      // use next-babel-loader instead of storybook's babel-loader
      if (
        rule.test.test('hello.js') &&
        Array.isArray(rule.use) &&
        rule.use[0].loader === 'babel-loader'
      ) {
        return false
      }
      return true
    }),
    ...nextWebpackConfig.module.rules.map((rule) => {
      // we need to resolve next-babel-loader since it's not available
      // relative with storybook's config
      if (rule.use && rule.use.loader === 'next-babel-loader') {
        rule.use.loader = require.resolve(
          'next/dist/build/webpack/loaders/next-babel-loader'
        )
      }
      if (rule.oneOf) {
        rule.oneOf = rule.oneOf
          .map((oneOfRule) => {
            // remove Next's error-loader rules
            if (oneOfRule.use && oneOfRule.use.loader === 'error-loader') {
              return null
            }
            // trim issuer
            if (oneOfRule.issuer && oneOfRule.issuer.and) {
              delete oneOfRule.issuer
            }
            // trim mini-css-extract-plugin
            if (Array.isArray(oneOfRule.use)) {
              oneOfRule.use = oneOfRule.use.filter(({loader}) => {
                return !/mini-css-extract-plugin/.test(loader)
              })
            }
            return oneOfRule
          })
          .filter(Boolean)
      }
      return rule
    }),
  ]

  return config
}

module.exports = {
  webpackFinal,
}

Currently I think the best workaround would be to remove the Next's oneOf rule completely and introduce own custom set of rules in Storybook.

@jnv
Copy link

jnv commented Aug 21, 2020

Okay, I have managed to make it work with global CSS by injecting a custom set of rules into that oneOf array, so it kinda work without rewriting all the CSS / CSS modules rules. I use the following Storybook config on top of the Next's SB plugin (note that this is Sass-specific but you can easily make it will work with CSS only):

// .storybook/main.js
const {sassOptions} = require('../next.config')
const path = require('path')

const includePath = path.resolve(__dirname, '../')

const cssRules = [
  {
    sideEffects: true,
    test: /(?<!\.module)\.css$/,
    use: ['style-loader', 'css-loader'],
    include: includePath,
  },
  {
    sideEffects: true,
    test: /(?<!\.module)\.(scss|sass)$/,
    use: [
      'style-loader',
      'css-loader',
      {
        loader: 'sass-loader',
        options: {
          sourceMap: true,
          sassOptions,
        },
      },
    ],
    include: includePath,
  },
]

module.exports = {
  stories: ['../src/**/*.stories.mdx', '../src/**/*.stories.@(js|jsx|ts|tsx)'],
  addons: [
    '@next/plugin-storybook',
    '@storybook/addon-links',
    '@storybook/addon-essentials',
  ],
  webpackFinal: async (config) => {
    const oneOfRule = config.module.rules.find((rule) => !!rule.oneOf)
    oneOfRule.oneOf = [...cssRules, ...oneOfRule.oneOf]
    return config
  },
}

@stefanprobst
Copy link
Contributor

to get the "Global CSS cannot be imported from files other than your Custom ." error instead of "Error: Module not found: Can't resolve 'error-loader'" maybe @next/plugin-storybook would need to also add resolveLoader from here to the webpack config in the storybook preset, i.e. config.resolveLoader = nextWebpackConfig.resolveLoader. this seems to work, however it does not print a nice code frame of the error.

@jankaifer jankaifer self-assigned this Dec 1, 2022
@jankaifer jankaifer added the please verify canary Please verify the issue with the latest canary branch. label Dec 1, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Dec 1, 2022

Please verify that your issue can be recreated with next@canary.

Why was this issue marked with the please verify canary label?

We noticed the provided reproduction was using an older version of Next.js, instead of canary.

The canary version of Next.js ships daily and includes all features and fixes that have not been released to the stable version yet. You can think of canary as a public beta. Some issues may already be fixed in the canary version, so please verify that your issue reproduces by running npm install next@canary and test it in your project, using your reproduction steps.

If the issue does not reproduce with the canary version, then it has already been fixed and this issue can be closed.

How can I quickly verify if my issue has been fixed in canary?

The safest way is to install next@canary in your project and test it, but you can also search through closed Next.js issues for duplicates or check the Next.js releases.

My issue has been open for a long time, why do I need to verify canary now?

Next.js does not backport bug fixes to older versions of Next.js. Instead, we are trying to introduce only a minimal amount of breaking changes between major releases.

What happens if I don't verify against the canary version of Next.js?

An issue with the please verify canary that receives no meaningful activity (e.g. new comments that acknowledge verification against canary) will be automatically closed and locked after 30 days.

If your issue has not been resolved in that time and it has been closed/locked, please open a new issue, with the required reproduction, using next@canary.

I did not open this issue, but it is relevant to me, what can I do to help?

Anyone experiencing the same issue is welcome to provide a minimal reproduction following the above steps. Furthermore, you can upvote the issue using the 👍 reaction on the topmost comment (please do not comment "I have the same issue" without repro steps). Then, we can sort issues by votes to prioritize.

I think my reproduction is good enough, why aren't you looking into it quicker?

We look into every Next.js issue and constantly monitor open issues for new comments.

However, sometimes we might miss one or two due to the popularity/high traffic of the repository. We apologize, and kindly ask you to refrain from tagging core maintainers, as that will usually not result in increased priority.

Upvoting issues to show your interest will help us prioritize and address them as quickly as possible. That said, every issue is important to us, and if an issue gets closed by accident, we encourage you to open a new one linking to the old issue and we will look into it.

Useful Resources

@balazsorban44
Copy link
Member

This issue has been automatically closed because it wasn't verified against next@canary. If you think it was closed by accident, please leave a comment. If you are running into a similar issue, please open a new issue with a reproduction. Thank you.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2023

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Easy to fix issues, good for newcomers please verify canary Please verify the issue with the latest canary branch.
Projects
None yet
Development

No branches or pull requests

7 participants