Support swagger module with AMD imports?

[chenatu]

I am using swagger-codegen to generate swagger js client. However, it is hard to import the client module.

Environment

1. node -v: 8.1.4
2. npm -v: 5.4.2
3. yarn --version (if you use Yarn): 1.1.0
4. npm ls react-scripts (if you haven’t ejected): 1.0.14

Then, specify:

1. Operating system: MAC OSX
2. Browser and version (if relevant): chrome

Steps to Reproduce

1. install swagger-codegen and generate javascript project according to some api spec
2. install local swagger module according to README.md file in the generated swagger project
3. import the module in the react project ant it shows:

Module not found: Can't resolve 'ApiClient' in /path/to/API

Expected Behavior

No error

Actual Behavior

In swagger issue 3537. It discusses this issue and bring 2 solutions:

Disabling directly edit the code source :

// commenting the first IF branch
  //if (typeof define === 'function' && define.amd) {
  //  // AMD. Register as an anonymous module.
  //  define(['superagent'], factory);
  //} else
  if (typeof module === 'object' && module.exports) {
    // CommonJS-like environments that support module.exports, like Node.
    module.exports = factory(require('superagent'));
  }

or via Webpack configs:

npm install --save-dev imports-loader
// and then add this loader to the webpack configs
{ test: /\.js/, loader: 'imports?define=>false'}

Since, it is not advised to revise the react-scripts, could create-react-app support swagger client imports?

[gaearon]

Sorry, we don't intend to support AMD. It's time the ecosystem moves forward from a format that isn't used by any system (neither Node nor browsers) and only exists as a legacy.

[john-goldsmith]

@gaearon Sorry for rehashing the past here, but it seems like the logical thread to pull with regards to this topic. While I absolutely agree with your statement above about not supporting AMD, my issue is centered around Swagger's ES6 generated output (specifically the usage of static class properties) and CRA's usage of related polyfills. Following the TC39 rabbit hole reveals:

Placement: Static vs instance -- static postponed to follow-on proposal

(source)

However, the CRA README says:

Supported Language Features and Polyfills

• Class Fields and Static Properties (part of stage 3 proposal).

While we recommend using experimental proposals with some caution, Facebook heavily uses these features in the product code, so we intend to provide codemods if any of these proposals change in the future.

Maybe I'm just overly concerned with the semantics, but the CRA README would lead me to believe that Swagger's usage of static class properties should play nicely with CRA, but my terminal says otherwise. At the same time, however, CRA not supporting an early Stage 2 proposal also seems to make sense. So, really, this is just a long-winded way of saying "I'm confused."

FWIW, you're obviously not responsible for Swagger's implementation. Given the incredible usefulness and popularity of both Swagger and React, having them live in harmony makes for a killer development setup.

[mjameswh]

Facing the same issue as the OP here.

Now, the original title of this issue is incorrect. Swagger's generated .js files does not require AMD modules support. It autodetect the proper modules semantics using a cascade of if statements. Now the problem is that by default, webpack loaders give hints that AMD modules are supported, though they are not really. Not sure about the details here, but in the end, since the generated code checks for AMD support before anything else, and is misled to believe that AMD is supported, then it initialize itself incorrectly.

The proposed fix is to tell webpack's module loader to completely disable support for AMD modules, by passing the argument define=>false to imports-loader. For example:

module: {
  rules: [
    {
      test: /my_client\/.*\.js$/,
      use: 'imports-loader?define=>false'
    }
  ]
}

This change seems to me as being compatible with CRA's policy regarding AMD modules... Could it be considered?

[gaearon]

Is there a simpler way to disable AMD support that doesn’t involve adding a loader? If not maybe you could propose this to webpack?

[mjameswh]

I could be wrong on that point, but from my understanding, the proposed configuration does not actually add the module, but only declares it explicitly with some parameter. I believe Webpack registers that same module implicitly if it is not declared.

Now about having a simpler way to disable AMD support... One way or another, that would still involve modifying the webpack configuration file, which, as of now, implies either ejecting from CRA or cloning the CRA script project...

[gaearon]

No, I’m suggesting that you propose this feature to webpack, and if webpack implements such an option, we’ll enable it by default in CRA. No ejecting.

[mezzario]

@gaearon Webpack implemented this feature:

https://webpack.js.org/configuration/module/#rule-parser

module: {
  rules: [
    {
      parser: {
        amd: false
      }
    }
  ]
}

@mjameswh you can generate ES6 javascript using swagger, set in config:

https://generator.swagger.io/api/gen/clients/javascript

{
  useES6: true
}

[seanlaff]

Using the useES6 flag in the swagger generation is a way to get around the AMD problem, however Im seeing the same errors @john-goldsmith saw about class fields
Support for the experimental syntax 'classProperties' isn't currently enabled

Which is odd because I know CRA supports that transform

[
        require("@babel/plugin-proposal-class-properties").default,
        {
          loose: true,
        },
      ],

[martnu]

@seanlaff When it comes to node_modules, CRA only compiles standard language features (not class properties). See #3776