swagger-ui icon indicating copy to clipboard operation
swagger-ui copied to clipboard

build: es6

Open tim-lai opened this issue 4 years ago • 9 comments
trafficstars

Description

create valid es modules instead of a es-bundle.

Motivation and Context

How Has This Been Tested?

  • works as 'npm link' to swagger-editor

  • does NOT work with proposed Jest test

Screenshots (if appropriate):

Checklist

My PR contains...

  • [x] 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).
  • [x] 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 avatar Dec 11 '20 22:12 tim-lai

"test:artifact:es" is not yet in CI test due to current error below

... and using import { resolve} from... yields same error.

    SyntaxError: Cannot use import statement outside a module

    > 1 | import resolve from "swagger-client/es/resolver"
        | ^
      2 | import { execute, buildRequest } from "swagger-client/es/execute"
      3 | import Http, { makeHttp, serializeRes } from "swagger-client/es/http"
      4 | import resolveSubtree from "swagger-client/es/subtree-resolver"

tim-lai avatar Dec 11 '20 22:12 tim-lai

update: this change will also necessitate changes to swagger-ui-react build, due to change in module path.

tim-lai avatar Dec 12 '20 00:12 tim-lai

The PR that changed how we use swagger-client in swagger-ui: https://github.com/swagger-api/swagger-ui/pull/6208

char0n avatar Dec 14 '20 14:12 char0n

My conclusion after analysis:

swagger-client issues

swagger-client issues will be resolved by adding transformIgnorePatterns: ['/node_modules/(?!swagger-client)', '\\.pnp\\.[^\\\/]+$'] by adding into jest config.

But this is not a solution to a problem. When swagger-client is out of the way, you will intercept many other similar errors. The reason is that es/ version of swagger-ui uses ES6 import to import other resources then JavaScript files; namely YAML files, images, styles etc... So using the same system for testing ES build fragments as used in swagger-client will not work here. Testing es/ build artifacts in swagger-client works because there are no imports to files except JavaScript files. In swagger-ui, completely different strategy needs to be utilized.

Proposed strategy of ES build fragment testing

In order to propertly test es/ build fragment, we need to bundle it to JavaScript bundle and then test the result of this bundling. Before the tests start, we'll use webpack to create UMD bundle from es/ build fragment (setup up phase). Then we test the UMD bundle it's default export is a function named SwaggerUI. After the test, we delete the UMD bundle (tear down). More proper test though would be an e2e test of above mentioned UMD bundle, verifying that it displays SwaggerUI.

char0n avatar Dec 14 '20 15:12 char0n

I'm pretty sure these changes alone won't produce a valid ESM bundle that can be consumed by other non-Webpack bundlers. The reason is that besides the already mentioned yaml and image imports swagger-ui also uses Webpack's require.context API which needs to be resolved at build time to be consumable by other bundlers. See for more details: https://github.com/swagger-api/swagger-ui/issues/6415#issuecomment-706697215

DreierF avatar Jan 17 '21 11:01 DreierF

@DreierF thanks for the comment.

You're right require.context is something that even e2e wouldn't catch because we'd use webpack to build the es/ fragments. I see two options, either get rid of require.context in code or use webpack to create one big es-bundle.js file which internally uses ES6 import statements to import vendor code from node_modules (tree-shakeable bundled code with ES6 imports).

char0n avatar Jan 17 '21 17:01 char0n

Hey folks, running into large bundle sizes for similar reasons. Anything I can do to help with getting this PR merged in?

gsdatta avatar Oct 08 '21 16:10 gsdatta

Just here to confirm this issue is the same when with next build If you're importing directly import X from "swagger-react-ui" if won't directly display the error message but using the dynamic api of next will give:

SyntaxError: Cannot use import statement outside a module
    at Object.compileFunction (node:vm:352:18)
    at wrapSafe (node:internal/modules/cjs/loader:1026:15)
    at Module._compile (node:internal/modules/cjs/loader:1061:27)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1149:10)
    at Module.load (node:internal/modules/cjs/loader:975:[32](https://gitlab-ncsa.ubisoft.org/maas/web/-/jobs/44414865#L32))
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:999:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (/node_modules/swagger-ui-react/commonjs.js:10:53)
    at Module._compile (node:internal/modules/cjs/loader:1097:14)

Just like the post of @tim-lai

davidroman0O avatar Mar 24 '22 16:03 davidroman0O

@davidroman0O did u find a solution?

discapes avatar Sep 06 '22 19:09 discapes

This has been solved in other PRs. Currently we have following package.json mapping:

  "main": "./dist/swagger-ui.js",
  "module": "./dist/swagger-ui-es-bundle-core.js",
  "exports": {
    "./dist/swagger-ui.css": "./dist/swagger-ui.css",
    "./dist/oauth2-redirect.html": "./dist/oauth2-redirect.html",
    "./dist/swagger-ui-standalone-preset": "./dist/swagger-ui-standalone-preset.js",
    ".": {
      "import": "./dist/swagger-ui-es-bundle-core.js",
      "require": "./dist/swagger-ui.js"
    }
  },

./dist/swagger-ui-es-bundle-core.js - is a pure ESM bundle.

char0n avatar Jun 12 '23 13:06 char0n