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

Webpack 5 support #1903

Merged
merged 12 commits into from
Jan 27, 2022
Merged

Webpack 5 support #1903

merged 12 commits into from
Jan 27, 2022

Conversation

juanca
Copy link
Contributor

@juanca juanca commented Oct 22, 2021

Motivation

Allow a host application with Webpack 5 to use react-styleguidist.

Technical changes

  • Upgraded react-dev-utils to latest version v12 (supports Webpack 5)
  • Fixed Webpack example for local testing
  • Upgraded webpack related dependencies for maintainence: webpack, webpack-cli, and webpack-dev-server
  • Removed unnecessary Webpack configuration around Hot Module Replacement. AFAIK, webpack-dev-server will automatically inject HMR related configurations already.

@artola

This comment has been minimized.

package.json Outdated Show resolved Hide resolved
@juanca
Copy link
Contributor Author

juanca commented Dec 3, 2021

If possible, I would like to get more 👀 on a webpack 5 compatible version. Can someone release a next version with these changes? If so, I can clean this up.

Let me know!

@juanca
Copy link
Contributor Author

juanca commented Dec 16, 2021

Heyo! v12 finally landed!. I'll get this updated, tested against my use-case, and then trim the artifacts (I don't really git-rebase so let me know if you need a pristine log history).

@@ -119,8 +119,6 @@ export default function (
}
} else {
webpackConfig = merge(webpackConfig, {
entry: [require.resolve('react-dev-utils/webpackHotDevClient')],
plugins: [new webpack.HotModuleReplacementPlugin()],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, this is automatically injected by the webpack dev server. My local dev environment seems to be auto-loading new code changes.

module.exports = {
title: 'React Style Guide Example',
components: 'src/components/**/*.js',
webpackConfig,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this example was broken and needed to include the Webpack config. I'm not sure if I ran the example incorrectly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be necessary: Styleguidist finds webpack.config.js in the project root.

@juanca
Copy link
Contributor Author

juanca commented Dec 21, 2021

This is ready for review.

  • I am not really sure what is going on with the CD pipeline.
  • Tests are passing locally.
  • Examples are working locally.
  • Test repository for integration testing is working in development and production.

@ajay-jindal

This comment has been minimized.

Copy link
Member

@sapegin sapegin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fixes!

Does it still work with webpack 4? If not, we should change the minimum version and release it as a major version.

module.exports = {
title: 'React Style Guide Example',
components: 'src/components/**/*.js',
webpackConfig,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be necessary: Styleguidist finds webpack.config.js in the project root.

@juanca
Copy link
Contributor Author

juanca commented Jan 14, 2022

This shouldn't be necessary: Styleguidist finds webpack.config.js in the project root.

I think this might be a side-effect of running the example in a nested directory:

node lib/bin/styleguidist.js server --config examples/webpack/styleguide.config.js

without an explicit Webpack configuration in the Styleguidist configuration results in:

./examples/webpack/src/components/Placeholder.js 10:18
Module parse failed: Unexpected token (10:18)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
|  */
| export default class Placeholder extends Component {
>       static propTypes = {
|               type: PropTypes.oneOf([
|                       'animal',
 @ ./lib/client/index.js (./lib/loaders/styleguide-loader.js!./lib/client/index.js) 44:30-130
 @ ./lib/client/index.js
 @ multi ./lib/client/index

@juanca
Copy link
Contributor Author

juanca commented Jan 14, 2022

Re: Webpack 4

I think it still works with Webpack ^4.46.0. Re-ran the example with i: no console errors and HMR was working.

image

@@ -24,7 +24,7 @@
"prop-types": "^15.6.1",
"react": "^17.0.1",
"react-dom": "^17.0.1",
"react-styleguidist": "^11.1.1",
"react-styleguidist": "file:../../",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this change to get the automatic Webpack configuration to work. This means you can run this particular package.json from its directory and it will treat itself as the root directory. This also means it will use the current working compiled code for react styleguidist when running this example. I think that's a better experience for developers because you will want to make changes to the library and test them against a local example. /cc @sapegin

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cd examples/webpack
npm install
npm run build
npm run styleguide

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome!

@juanca
Copy link
Contributor Author

juanca commented Jan 14, 2022

If we're okay with the existing change set, I can go ahead and fix the merge conflicts.

Copy link
Member

@sapegin sapegin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍨

 Conflicts:
	examples/webpack/package-lock.json
@juanca
Copy link
Contributor Author

juanca commented Jan 26, 2022

@sapegin If you make a pre-release, I can test it out on my repository as well.

@sapegin sapegin merged commit 92518df into styleguidist:master Jan 27, 2022
@sapegin
Copy link
Member

sapegin commented Jan 27, 2022

Making a prerelease is way too much work for me (and I dont't have neither time nor motivation to do any open source now), so I'm merging it as is and praying that semantic-release still works ;-)

Thanks for the fix! 🤗

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

Successfully merging this pull request may close these issues.

4 participants