-
Notifications
You must be signed in to change notification settings - Fork 9k
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
feat(build): webpack@5 and webpack-dev-server@4 #7826
feat(build): webpack@5 and webpack-dev-server@4 #7826
Conversation
* touch swagger-ui-core files for dist
* replace with fully defined babel config
* this change requires webpack@5
…webpack-dev-server@4-umd-only
…webpack-dev-server@4-umd-only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've provided deep code review. All my suggestions are also tested locally.
…webpack-dev-server@4-umd-only
@char0n Update on npm link:
Error log:
|
@char0n per request, I pushed up a duplicate new branch that may be easier for you to work with: https://github.com/swagger-api/swagger-ui/tree/feat/wp5 |
f4ab6c7
to
4a923cf
Compare
* replaces file-loader, raw-loader, url-loader
…webpack-dev-server@4-umd-only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could verify that build fragments work along with swagger-ui-react. Left bunch of final code review comments. Overall build system works.
…webpack-dev-server@4-umd-only
Definitely did not fix #7704, exactly the same error still. Please make sure you reproduce the original error, or wait for a confirmation of issue being resolved, before closing an issue. |
In the current version, the OpenApi documentation cannot be displayed, because "Buffer is not defined". This is due to an update to Webpack 5 from November 2021 that removed automatic Node.js polyfills. In version 4.11.x of swagger-ui, Webpack was also bumped to version 5 and polyfills were loaded separately as fallback. See swagger-api/swagger-ui#7826
SwaggerUI is now built using `webpack@5`, with dev support for `webpack-dev-server@4` - ES Module output bundle path now points to `swagger-ui-es-bundle-core`, which does not include dependencies - No change to CommonJS output bundle or path - Now uses Asset Modules, which replaces `file-loader`, `raw-loader`, and `url-loader` - Removed unused rules/loaders for `.woff | .woff2 | .ttf | .eot` fonts and html - Node polyfills are no longer bundled with `webpack@5`, and must be loaded separately and/or use `resolve.fallback`. As an example, SwaggerUI loads `process`, `buffer`, and `stream-browserify` as `devDependencies` in order to build development and production bundles. SwaggerUI-React - Now imports `swagger-ui-es-bundle-core`, and similarly outputs `swagger-ui-es-bundle-core` to its `dist` directory Dev notes: - Order of execution matters for the production npm build scripts. `build-stylesheets` needs to get built first, then cleanup of any empty artifacts, before building the various production bundles - `Dev-helpers` now relies on `HTMLWebpackPlugin` to inject css and bundle files
Description
Cumulative changes to enable
webpack@5
withwebpack-dev-server@4
.swagger-ui-bundle
).npm run dev
loads webpack-dev-server@4 with hot reloading.swagger-ui-react
. Verified.swagger-ui-dist
. No change@pmmmwh/react-refresh-webpack-plugin
.Note, library module now maps to
swagger-ui-es-bundle-core
instead ofswagger-ui-es-bundle
, which means that dependencies are no longer bundled as part of the npm package. This change should help enable better tree-shaking, as well as avoiding errors about multiple React versions that use React hooks (Ref React Hook error).Similarly,
swagger-ui-react
library module now includesswagger-ui-es-bundle-core
instead ofswagger-ui-es-bundle
. This change should help alleviate issues when runningswagger-ui-react
withcreate-react-app@5
, which was complaining of multiple react versions being loaded.This PR does not address true ESM output in either SwaggerUI or SwaggerUI-React.
Motivation and Context
Replaces #7788, which further changed SwaggerUI and
swagger-ui-es-bundle-core
to be true ESMAlso replaces #7349
fixes #7704
How Has This Been Tested?
see checklist from description above
Screenshots (if appropriate):
Checklist
My PR contains...
src/
is unmodified: changes to documentation, CI, metadata, etc.)package.json
)My changes...
Documentation
Automated tests