Open char0n opened 2 years ago
re: phase 1, swagger-ui.js
containing both SwaggerUI
and SwaggerUICore... by including both exports, will this increase the bundle size? Generally, the community is asking for smaller bundle size with
swagger-ui-bundle.js`, and we have intermittently bumped up the self-imposed max size webpack error check to accommodate new features and/or replacement libraries.
re: cjs/mjs
file extensions... there seems to be many reports of compatibility issues with mixing exports across libraries, as well as a possible lack of long term adoption for mjs
. So it might be safer to continue to use package.esm.js/package.umd.js
format that also describes build settings.
I'm definitely in favor of bringing swagger-ui-react
into the primary build workflow. However, we might want to keep the npm package name available.
One other consideration is swagger-ui-dist
. Like swagger-ui-react
, I think it would be good to maintain the npm package, but perhaps also include swagger-ui-react
.
re: phase 1, swagger-ui.js containing both SwaggerUI and SwaggerUICore... by including both exports, will this increase the bundle size?
No, it will only increase overall size of npm
package size, which very few people care about. What people care about is having proper build fragments and having them mapped properly.
Generally, the community is asking for smaller bundle size with swagger-ui-bundle.js`, and we have intermittently bumped up the self-imposed max size webpack error check to accommodate new features and/or replacement libraries.
This is going to be solved by swagger-ui.mjs
- the true ESM build will all the externals referenced via import
statement. Then it's up to consumer bundling mechanism to fully benefit from it (tree shaking, dead code elimination, code splitting).
re: cjs/mjs file extensions... there seems to be many reports of compatibility issues with mixing exports across libraries,
I'm not sure I understand this argument. Compatibility issues arises from the fact that npm ecosystem is moving towards pure ESM and want's to kill CommonJS and not from the fact how extensions are named.
as well as a possible lack of long term adoption for mjs.
What make you said that? When SwaggerUI in converted to ESM package (by setting "type": "module"
in package.json), then the every file within the repository will be implicitly considered mjs
. That make it possible to still name file with *.js
extension which is at that point equivalent with *.mjs
.
So it might be safer to continue to use package.esm.js/package.umd.js format that also describes build settings.
The benefit of proposed naming is that we retain backward compatibility - by having swagger-ui.js
which is UMD build and having additional build fragments swagger-ui.mjs
and swagger-ui.cjs
which exactly communicate the format of the bundle. *.cjs
will be required to have after the SwaggerUI is converted to ESM package (by setting "type": "module"
in package.json) to support CommonJS fragments as well. Without .cjs extension you will not be able to use with require
function.
I'm definitely in favor of bringing swagger-ui-react into the primary build workflow. However, we might want to keep the npm package name available.
Can you elaborate what you mean by "keeping the package name available"?
One other consideration is swagger-ui-dist. Like swagger-ui-react, I think it would be good to maintain the npm package, but perhaps also include swagger-ui-react.
I'm not sure I understand. Are you saying you want retain swagger-ui-dist
and making what is now swagger-ui-react
part of it? What would be the benefit and why do you think it's something we should do?
The overall goal of unifying the npm packages is to limit the complexity of release process.
swagger-ui.js containing both SwaggerUI and SwaggerUICore
Ok, re-reading this, the proposal is to include two symbols on the global object within swagger-ui.js
instead of just one, and it now includes dependencies. The original swagger-ui.js
non-dependency bundle is renamed to the proposed swagger-ui.cjs
.
swagger-ui-react
My comment was that it would be nice to build the swagger-ui-react
files when we build the other bundles. This would leverage the same build system. Then the swagger-ui-react
script only needs to copy the files (and updated package.json) it needs for npm distribution.
.mjs/.cjs
filename conventions
My main concern is around possible support issues around the proposed change on this specific point, and how consumers access and use the artifacts we make available. E.g. downstream projects, Next.js, browser/non-browser etc.
For example, I was trying to envision scenarios where users may be directly importing a file within a package, e.g. Docker script uses dist/index.html
, where it would be nice to limit the changes required so that users would not need to change their own scripts.
swagger-ui-react + swagger-ui-dist
This was just an idea, where swagger-ui-dist also included the swagger-ui-react bundle files (not module). The name of swagger-ui-dist
suggests a "complete" set of files that could be used. Upon further thought, it's not something I'd really want, as my preference is to encourage usage of esm rather than inlining specific file bundles. And I view swagger-ui-dist
as a package of browser bundles.
This is current state of our build fragments
This document describe state after https://github.com/swagger-api/swagger-ui/pull/7826 is merged.
swagger-ui.js
This is UMD build that doesn't include production dependencies and exports
SwaggerUICore
symbol on global object.swagger-ui-bundle.js
This is UMD build that does include production dependencies and exports
SwaggerUIBundle
symbol on global object.swagger-ui-standalone-preset.js
This is UMD build that does include production dependencies and exports
SwaggerUIStandalonePreset
symbol on global object.swagger-ui-es-bundle.js
This is
commonjs2
build that does include production dependencies.swagger-ui-es-bundle-core.js
This is true
ESM
bundle that doesn't include production dependencies.The difference between
commonjs
andcommonjs2
:commonjs
mean pure CommonJs (less flexibility)commonjs2
also includes themodule.exports
stuff.Proposal
I propose to simplify our build system and build fragments following backwards compatible way:
swagger-ui.js
This is UMD build that does include production dependencies and exports
SwaggerUI
symbol on global object. For backward compatibility we should also make sure that this bundle exportsSwaggerUICore
as well on global object.swagger-ui-standalone.js
This is UMD build that does include production dependencies and exports
SwaggerUIStandalonePreset
symbol on global object. No change here.swagger-ui.mjs
This is true ESM build that doesn't include production dependencies. This can be achieved by using following webpack config.
swagger-ui.cjs
This is
commonjs2
build that doesn't include production dependencies. This will be deprecated, exists mostly for backward compatible reasons and will be removed in next major release following the next major release (two major releases) of swagger-ui.package.json mappings
To achieve backward compatibility I propose following mappings:
Proposal phase 2
In this phase we should consider using forked CRA for our build system as demonstrated here so that we don't need to maintain a lot of webpack/babel/eslint configuration our selves and benefit from work of Facebook team which work hard of embedding current industry standards into the CRA build system. The only drawback is that we're now be able to easily update build deps like webpack/eslint/babel unless CRA does so. But there is still en option for ejecting if it comes to that.
This is suitable doing in next breaking change release of swagger-ui. First step is converting
swagger-ui
to pure ESM module first. This will allow to comsume other ESM modules which entire npm ecosystem is now converging to.Build fragments will change to following:
swagger-ui.js
This is UMD build that does include production dependencies and exports
SwaggerUI
symbol on global object. For backward compatibility we should also make sure that this bundle exportsSwaggerUICore
as well on global object. This fragments is used mostly via unpkg.com.swagger-ui-standalone.js
This is UMD build that does include production dependencies and exports
SwaggerUIStandalonePreset
symbol on global object. No change here.swagger-ui.mjs
This is true ESM build that doesn't include production dependencies. This can be achieved by using following webpack config.
swagger-ui.cjs
This is
commonjs2
build that doesn't include production dependencies. This will be deprecated, exists mostly for backward compatible reasons and will be removed in next major release following the next major release (two major releases) of swagger-ui.Following mappings should be introduced:
We're dropping old mappings and using new
exports
field exclusively. Our support for particularNode.js
versions should reflect that.exports
fields allows to maps submodules so we can directly map the swagger-ui react component as well.swagger-ui-react
andswagger-ui-dist
can be discontinued. This also requires chandingreact
andreact-dom
to be peer dependencies. Versions ofNode.js
that supportsexports
field also support installing peer deps implicitly.From
To