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

Disable AMD #6018

Closed
wants to merge 1 commit into from
Closed

Disable AMD #6018

wants to merge 1 commit into from

Conversation

martnu
Copy link

@martnu martnu commented Dec 11, 2018

Disable AMD and solve swagger client issues since there is no intention of supporting it:

#3247 (comment)

Disable AMD and solve swagger client issues since there is no intention of supporting it:

facebook#3247 (comment)
@martnu martnu changed the title Update webpack.config.js Disable AMD Dec 11, 2018
@Timer
Copy link
Contributor

Timer commented Dec 11, 2018

I've also seen an alternative way to disable AMD: https://github.com/webpack-contrib/imports-loader#disable-amd

Can you please explain the difference between the two?

p.s. thanks for the PR!

@Timer Timer added this to the 3.0 milestone Dec 11, 2018
@martnu
Copy link
Author

martnu commented Dec 11, 2018

@Timer Well, disabling it via module.rules.parser disables the entire AMD-plugin for webpack. (https://github.com/webpack/webpack/blob/ed9691585e5f5e8d8a6c5985465b85f5ea265011/lib/dependencies/AMDPlugin.js#L101)

While doing it via imports-loader sets the define-variable to false which is used to check for AMD in most UMD-modules. See https://github.com/umdjs/umd/blob/master/templates/commonjsStrict.js#L20

@Timer
Copy link
Contributor

Timer commented Dec 11, 2018

I just want to make sure we can still load a UMD module that specifies AMD and it's not broken using this proposed option. Can we add an e2e test for this, please?

@martnu martnu closed this Feb 17, 2019
@martnu martnu deleted the patch-1 branch February 17, 2019 16:20
@Timer
Copy link
Contributor

Timer commented Feb 18, 2019

Hi @martnu! Could you please tell us why you closed this?

@martnu
Copy link
Author

martnu commented Feb 18, 2019

I suggest using another client generation language for swagger, for instance typescript-fetch. This will generate a commonjs-friendly client.

@Timer
Copy link
Contributor

Timer commented Feb 18, 2019

Sure, but we still want to disable AMD for other users!

@lock lock bot locked and limited conversation to collaborators Feb 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants