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

SwaggerUI build fragments proposal #7831

Open
char0n opened this issue Feb 9, 2022 · 3 comments
Open

SwaggerUI build fragments proposal #7831

char0n opened this issue Feb 9, 2022 · 3 comments

Comments

@char0n
Copy link
Member

char0n commented Feb 9, 2022

This is current state of our build fragments

This document describe state after #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 and commonjs2:

commonjs mean pure CommonJs (less flexibility)
commonjs2 also includes the module.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 exports SwaggerUICore 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:

  "main": "./dist/swagger-ui.cjs",
  "module": "./dist/swagger-ui.mjs",
  "exports": {
    ".": {
      "browser": "./dist/swagger-ui.mjs",
      "node": "./dist/swagger-ui.cjs"
      "import": "./dist/swagger-ui.mjs",
      "require": "./dist/swagger-ui.js",
    }
  }

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 exports SwaggerUICore 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:

  "exports": {
   "./react": {
      "browser": "./flavors/swagger-ui-react/index.mjs",
      "node": "./flavors/swagger-ui-react/commonjs.cjs"
      "import": "./flavors/swagger-ui-react/index.mjs",
      "require": "./flavors/swagger-ui-react/commonjs.cjs",    
   },
    ".": {
      "browser": "./dist/swagger-ui.mjs",
      "node": "./dist/swagger-ui.cjs"
      "import": "./dist/swagger-ui.mjs",
      "require": "./dist/swagger-ui.cjs",
    }
  }

We're dropping old mappings and using new exports field exclusively. Our support for particular Node.js versions should reflect that. exports fields allows to maps submodules so we can directly map the swagger-ui react component as well.

import SwaggerUIReact from 'swagger-ui/react';

swagger-ui-react and swagger-ui-dist can be discontinued. This also requires chanding react and react-dom to be peer dependencies. Versions of Node.js that supports exports field also support installing peer deps implicitly.

From

"depenencies": {
    "react": "=17.0.2",
    "react-dom": "=17.0.2"
}

To

"peerDepenencies": {
    "react": "=17.0.2",
    "react-dom": "=17.0.2"
}
@tim-lai
Copy link
Contributor

tim-lai commented Feb 9, 2022

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.

@char0n
Copy link
Member Author

char0n commented Feb 15, 2022

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.

@tim-lai
Copy link
Contributor

tim-lai commented Feb 16, 2022

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.

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

No branches or pull requests

2 participants