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

feat(build): webpack@5 and webpack-dev-server@4 #7826

Merged

Conversation

tim-lai
Copy link
Contributor

@tim-lai tim-lai commented Feb 3, 2022

Description

Cumulative changes to enable webpack@5 with webpack-dev-server@4.

  • swagger-ui (umd). Not directly verified, but the bundled version with dependencies is validated to work (swagger-ui-bundle).
  • swagger-ui-bundle (umd). Verified. browser tested via static build script
  • swagger-ui-es-bundle-core (commonjs2). Verified. Can import library from separate project.
  • swagger-ui-es-bundle (commonjs2). Verified. Can import library from separate project.
  • swagger-ui (css). Verified. browser tested with static, dev, and dev-e2e build scripts
  • dev bundle. Verified. npm run dev loads webpack-dev-server@4 with hot reloading.
  • dev bundle for Cypress CI. Verified. cypress tests pass
  • updated for swagger-ui-react. Verified.
  • updated for swagger-ui-dist. No change
  • dev-helpers. Verified. HMR and react hot reloading via @pmmmwh/react-refresh-webpack-plugin.

Note, library module now maps to swagger-ui-es-bundle-core instead of swagger-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 includes swagger-ui-es-bundle-core instead of swagger-ui-es-bundle. This change should help alleviate issues when running swagger-ui-react with create-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 ESM
Also replaces #7349

fixes #7704

How Has This Been Tested?

see checklist from description above

Screenshots (if appropriate):

Checklist

My PR contains...

  • No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes to a public API (config options, System API, major UI change, etc).
  • are breaking changes to a private API (Redux, component props, utility functions, etc.).
  • are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.
  • My changes require a change to the project documentation.
  • If yes to above: I have updated the documentation accordingly.

Automated tests

  • My changes can not or do not need to be tested.
  • My changes can and should be tested by unit and/or integration tests.
  • If yes to above: I have added tests to cover my changes.
  • If yes to above: I have taken care to cover edge cases in my tests.
  • All new and existing tests passed.

@tim-lai tim-lai requested a review from char0n February 4, 2022 00:02
Copy link
Member

@char0n char0n left a 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/stylesheets.babel.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
webpack/dev.babel.js Show resolved Hide resolved
webpack/_config-builder.js Outdated Show resolved Hide resolved
webpack/_config-builder.js Outdated Show resolved Hide resolved
webpack/_config-builder.js Outdated Show resolved Hide resolved
webpack/es-bundle-core.babel.js Outdated Show resolved Hide resolved
@tim-lai
Copy link
Contributor Author

tim-lai commented Feb 17, 2022

@char0n Update on swagger-ui-react with swagger-ide

npm link:

➜  swagger-ide git:(main) ✗ npm ls swagger-ui-react
@swagger-api/swagger-ide@0.4.0 /Users/tim/Repos/swagger-ide
└── swagger-ui-react@4.5.0 -> ./../swagger-ui/flavors/swagger-ui-react/dist

Error log:

Failed to compile.

Module not found: Error: Can't resolve 'url' in '/Users/tim/Repos/swagger-ide/node_modules/@apidevtools/json-schema-ref-parser/lib/util'
Did you mean './url'?
Requests that should resolve in the current directory need to start with './'.
Requests that start with a name are treated as module requests and resolve within module directories (node_modules, /Users/tim/Repos/swagger-ide/node_modules).
If changing the source code is not an option there is also a resolve options called 'preferRelative' which tries to resolve these kind of requests in the current directory too.

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
	- add a fallback 'resolve.fallback: { "url": require.resolve("url/") }'
	- install 'url'
If you don't want to include a polyfill, you can use an empty module like this:
	resolve.fallback: { "url": false }
0.99 done plugins webpack-dev-server
0.99 done plugins
0.99
0.99 cache store build dependencies
0.99 cache store build dependencies IdleFileCachePlugin
0.99 cache store build dependencies
0.99 cache begin idle
0.99 cache begin idle IdleFileCachePlugin
0.99 cache begin idle
1
assets by status 34.9 MiB [cached] 9 assets
assets by path . 3.03 KiB
  asset index.html 1.82 KiB [emitted]
  asset asset-manifest.json 1.21 KiB [emitted]
cached modules 21.2 MiB (javascript) 424 KiB (asset) 91.9 KiB (runtime) [cached] 5076 modules
../swagger-ui/flavors/swagger-ui-react/dist/index.js 7.83 KiB [built]

ERROR in ./node_modules/@apidevtools/json-schema-ref-parser/lib/util/url.js 13:0-36
Module not found: Error: Can't resolve 'url' in '/Users/tim/Repos/swagger-ide/node_modules/@apidevtools/json-schema-ref-parser/lib/util'
Did you mean './url'?
Requests that should resolve in the current directory need to start with './'.
Requests that start with a name are treated as module requests and resolve within module directories (node_modules, /Users/timothy.lai/Repos/swagger-ide/node_modules).
If changing the source code is not an option there is also a resolve options called 'preferRelative' which tries to resolve these kind of requests in the current directory too.

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
	- add a fallback 'resolve.fallback: { "url": require.resolve("url/") }'
	- install 'url'
If you don't want to include a polyfill, you can use an empty module like this:
	resolve.fallback: { "url": false }
 @ ./node_modules/@apidevtools/json-schema-ref-parser/lib/index.js 16:12-33
 @ ./node_modules/@asyncapi/parser/lib/parser.js 9:19-65
 @ ./node_modules/@asyncapi/parser/lib/index.js 1:15-34
 @ ./src/components/AsyncApiComponent.jsx 10:0-41 29:8-13
 @ ./src/plugins/asyncapi-react/index.js 5:0-71 12:6-23
 @ ./src/App.jsx 17:0-63 48:67-81
 @ ./src/index.jsx 9:0-28 17:38-41

ERROR in ../swagger-ui/flavors/swagger-ui-react/dist/index.js 64:15-35
export 'default' (imported as 'swaggerUIConstructor') was not found in './swagger-ui-es-bundle-core' (module has no exports)
 @ ./src/App.jsx 10:0-41 33:36-45 50:23-42 51:31-53
 @ ./src/index.jsx 9:0-28 17:38-41

ERROR in ../swagger-ui/flavors/swagger-ui-react/dist/index.js 71:53-65
export 'presets' (imported as 'presets') was not found in './swagger-ui-es-bundle-core' (module has no exports)
 @ ./src/App.jsx 10:0-41 33:36-45 50:23-42 51:31-53
 @ ./src/index.jsx 9:0-28 17:38-41

ERROR in ./node_modules/@swagger-api/apidom-reference/es/util/url.js 10:66-73
Module not found: Error: Can't resolve 'process/browser' in '/Users/tim/Repos/swagger-ide/node_modules/@swagger-api/apidom-reference/es/util'
Did you mean 'browser.js'?
BREAKING CHANGE: The request 'process/browser' failed to resolve only because it was resolved as fully specified
(probably because the origin is strict EcmaScript Module, e. g. a module with javascript mimetype, a '*.mjs' file, or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.
 @ ./node_modules/@swagger-api/apidom-reference/es/index.js 2:0-37 35:23-43 35:51-73
 @ ./node_modules/@swagger-api/apidom-ls/es/services/deref/deref-service.js 2:0-66 53:31-48
 @ ./node_modules/@swagger-api/apidom-ls/es/apidom-language-service.js 6:0-72 21:27-46
 @ ./node_modules/@swagger-api/apidom-ls/es/index.js 2:0-77 2:0-77
 @ ./src/plugins/monaco/workers/apidom/ApiDOMWorker.js 7:0-70 17:16-29 19:28-46
 @ ./src/plugins/monaco/workers/apidom/apidom.worker.js 5:0-49 9:15-27

@tim-lai
Copy link
Contributor Author

tim-lai commented Feb 17, 2022

@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

@tim-lai tim-lai force-pushed the feat/webpack-dev-server@4-umd-only branch from f4ab6c7 to 4a923cf Compare February 24, 2022 21:45
* replaces file-loader, raw-loader, url-loader
webpack/dev.babel.js Outdated Show resolved Hide resolved
webpack/dev.babel.js Outdated Show resolved Hide resolved
webpack/es-bundle.babel.js Outdated Show resolved Hide resolved
Copy link
Member

@char0n char0n left a 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.

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
webpack/_config-builder.js Outdated Show resolved Hide resolved
webpack/dev.babel.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@haavardw
Copy link

haavardw commented Mar 12, 2022

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.

Pilopa pushed a commit to scm-manager/scm-openapi-plugin that referenced this pull request Jun 1, 2022
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

swagger-ui-react: Build broken with react-scripts 5.0.0
3 participants