styleguidist / react-styleguidist

Isolated React component development environment with a living style guide
https://react-styleguidist.js.org/
MIT License
10.83k stars 1.44k forks source link

Error when running build: "Cannot read property 'endsWith' of undefined" #1247

Closed jgalazm closed 5 years ago

jgalazm commented 5 years ago

Current behavior When trying to build the styleguide with npx styleguidist build after create-react-app and copying the cra example components folder I'm getting this error:

Building style guide...
Cannot read property 'endsWith' of undefined

To reproduce From my fork repository: https://github.com/jgalazm/react-styleguidist

run

cd examples/cra2
npm install
npx styleguidist build

and you will get the error

Building style guide...
Cannot read property 'endsWith' of undefined

From scratch, inside the repo's folder run:

cd examples
create-react-app cra2
cd cra2
npm install
npm install --save-dev react-styleguidist

Then create styleguide.config.js with this content (see #1243)

module.exports = { 
    webpackConfig: require( 'react-scripts/config/webpack.config.js' )
}

Copy the components from the cra example folder:

cp -r ../cra/src/components/ src/.

And build it:

npx styleguidist build

Here is when the error appears:

Building style guide...
Cannot read property 'endsWith' of undefined

Expected behavior After running

npm install
npx styleguidist build

To have the static files located in the styleguide folder. This already works for the original cra/ example.

osbornm commented 5 years ago

I'm running into this same problem...

osbornm commented 5 years ago

TL:DR

put the following add the following dangerouslyupdatewebpackconfig to your styleguide.config.js file

dangerouslyUpdateWebpackConfig(webpackConfig, env) {
    webpackConfig.output = {
        ...webpackConfig.output,
        publicPath: process.env.PUBLIC_URL || ''
    };
    return webpackConfig;
}

Root Cause

Okay, I've tracked down the problem. With the newer version of create-react-app they use react-dev-utils and this is where the "problem" comes from. Some of the helper utils in that make some assumptions about the webpack config that are always true for their "supported" use cases. This particular problem comes from assuming there is a publicPath property, basically, they are not null checking because in there use cases this is autogenerated if not provided.

Styleguidist does take the output webpack config from react-scripts/config/webpack.config.js and merge it will it's own. Doing addresses most of these types of issues but it expelicitly ignores the output section.

Fix

The fix for this is that styleguidist should include a publicPath property in the output section of the webpack config using process.env.PUBLIC_URL if present or \ if not. I'm happy to submit a PR if @sapegin or similar are happy with this approach.

Workaround

until a fix is in you can use the dangerousltUpdateWebpackConfig method to solve this. your styleguide.config.js will look something like

module.exports = {
    webpackConfig: require('react-scripts/config/webpack.config.js'),
    dangerouslyUpdateWebpackConfig(webpackConfig, env) {
        webpackConfig.output = {
            ...webpackConfig.output,
            publicPath: process.env.PUBLIC_URL || ''
        };
        return webpackConfig;
    }
}
sapegin commented 5 years ago

Thanks for the investigation! I have some concerns on passing the app’s path or /. Will this work fine when the style guide is deployed to a folder? For example http://example.com/styleguide/.

osbornm commented 5 years ago

@sapegin I'll need to verify but create-react-app "expects" this pattern with its config so if you are using their webpack config you should be okay. I dug through all the code paths to figure out what publicPath was getting set to. That said if you needed to change to a different path you could do something like PUBLIC_PATH=/foo styleguide build in your package.json. That said styleguidist could also just offer a config option and then just default to / if nothing is provided.

sapegin commented 5 years ago

Requiring users to add a new config value for something that's already working without isn't an option.

Can we pass something like an empty string?

osbornm commented 5 years ago

I'm not suggesting a required option just the ability to add one if you need since it's dependent on the webpack config you are using. This way instead of having to use dangerouslyUpdateWebpackConfig you would just set an optional config called, I don't know, publicPath. If the user sets it then we use that otherwise set it to empty string.

module.exports = {
    webpackConfig: require('react-scripts/config/webpack.config.js'),
    publicPath: 'styleguide/'
}

then in make-webpack-config.js L37-L41 would become

output: {
  path: config.styleguideDir,
  filename: 'build/[name].bundle.js',
  chunkFilename: 'build/[name].js',
  publicPath: config.publicPath || ''
},
sapegin commented 5 years ago

Again: you’re suggesting to introduce an option for a feature that already works out of the box.

osbornm commented 5 years ago

It does not work out of the box with the latest versions of create-react-app you have to manaully write a webpack config and you cannot take advantage of the webpack config that ships with the app. it is perfectly acceptable if you do not want to support create-react-app.

please see #1243 for the work around that allows you to point styleguidist at the webpack config that now ships with create-react-app. Doing this will allow you to run a development version but will not allow you to build aka the source of this bug

simonedavico commented 5 years ago

I also have this issue, styleguidist server with the latest react-scripts runs ok, but styleguidist build produces the above error.

I agree with @osbornm, as of today it does not work out of the box.

lucifer1004 commented 5 years ago

@simonedavico

The difference between styleguidist server and styleguidist build is the webpackEnv variable, which is development when using styleguidist server, and production when using styleguidist build.

I have found another workaround, that is to set

webpackConfig: require('react-scripts/config/webpack.config')('development'),

in styleguide.config.js.

This will force webpackEnv to be development, so that build can succeed.

Currently, I have not found any issues with this workaround. React Developer Tool does not give any warning on the generated pages.

simonedavico commented 5 years ago

@lucifer1004 thank you, it works! Not sure what is the implication of always forcing the development config though.

osbornm commented 5 years ago

@simonedavico you can use my suggested workaround and then you wont be in DEV all the time. I have a fork of a fix that does this for folks. HOWEVER, @sapegin claims this is not an actually issue and has stoped responding to my comments.

styleguidist-bot commented 5 years ago

:tada: This issue has been resolved in version 9.0.6 :tada:

The release is available on:

Your semantic-release bot :package::rocket: