Skip to content

Adjust settings to uglifyjs mangler to allow CJS and UMD bundles to build correctly. #6

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

Merged
merged 1 commit into from
Apr 11, 2018

Conversation

pl12133
Copy link
Contributor

@pl12133 pl12133 commented Apr 11, 2018

Keep the _createClass name to avoid it being removed by dead code elimination.
Keep function names because graphql relies on them.

… build correctly.

Keep the "_createClass" function name to avoid it being removed by dead code elimination.
Keep function names because `graphql` relies on them.
@billneff79
Copy link
Contributor

Do you need a new version released for this now, or do we want to wait for the next necessary version push?

@billneff79 billneff79 merged commit e508a51 into master Apr 11, 2018
@pl12133
Copy link
Contributor Author

pl12133 commented Apr 12, 2018

I'm not aware of anyone being effected by this broken build problem so it doesn't need to be in an immediate release.

@pl12133
Copy link
Contributor Author

pl12133 commented Apr 12, 2018

It also seems like using the CJS or UMD bundles will trigger graphql/graphql-js#594 in the zm-x-web application, presumably because there are two versions of graphql: the version bundled into @zimbra/api-client conflicting with the version existing in the main zm-x-web app. I could be wrong because I haven't taken much time to investigate.

Since building from the ESM bundle can deduplicate the imports, it doesn't show the same problem; and that makes it a low priority to fix so I will not be looking into it further until someone presents it as a real problem.

@silentsakky silentsakky deleted the chore/fix-build branch August 7, 2018 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants