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

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

Closed
jgalazm opened this issue Jan 11, 2019 · 13 comments
Closed

Comments

@jgalazm
Copy link

jgalazm commented Jan 11, 2019

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
Copy link

osbornm commented Jan 15, 2019

I'm running into this same problem...

@osbornm
Copy link

osbornm commented Jan 16, 2019

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
Copy link
Member

sapegin commented Jan 16, 2019

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
Copy link

osbornm commented Jan 16, 2019

@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
Copy link
Member

sapegin commented Jan 16, 2019

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
Copy link

osbornm commented Jan 16, 2019

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
Copy link
Member

sapegin commented Jan 16, 2019

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

@osbornm
Copy link

osbornm commented Jan 16, 2019

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
Copy link

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
Copy link
Contributor

@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
Copy link

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

@osbornm

This comment has been minimized.

@styleguidist styleguidist locked as too heated and limited conversation to collaborators Feb 26, 2019
@sapegin sapegin closed this as completed in 6663744 Apr 8, 2019
@styleguidist-bot
Copy link
Collaborator

🎉 This issue has been resolved in version 9.0.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants