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

Add OMIT_MODULE_EXPORTS to omit module code #12952

Closed
wants to merge 3 commits into from

Conversation

kjlubick
Copy link
Contributor

@kjlubick kjlubick commented Dec 3, 2020

This export code is the root cause of flutter/flutter#58428 so we would like to be able to remove it.

@welcome
Copy link

welcome bot commented Dec 3, 2020

Thank you for submitting a pull request! If this is your first PR, make sure to add yourself to AUTHORS.

@sbc100
Copy link
Collaborator

sbc100 commented Dec 3, 2020

I would love to find a way to do this without adding a new settings if we can. However, I'm not familiar enough with MODULARIZE or the its users. Maybe there are other options such as completely removing the UMD style export stuff?

Does the EXPORT_ES6 mode also cause you problems?

If we are going to add a new option I think we need a better name because both MODULE and EXPORT are pretty overloaded here. Maybe something more specific like NO_UMD_EXPORTS?

@RReverser has been battling with modularize recently so might have some insight into the right direction to go here.

@kjlubick
Copy link
Contributor Author

kjlubick commented Dec 3, 2020

ES6 modules do work for the browser, but they make use on node.js more problematic (unless we want multiple builds, one for node, one for web).

I'm fine with renaming the option to NO_UMD_EXPORTS so as not to be confusing with the Module object.

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kjlubick For your use case, can you build with EXPORT_ES6 (which would avoid emitting UMD code)?

emcc.py Outdated Show resolved Hide resolved
@kripken
Copy link
Member

kripken commented Dec 4, 2020

Oh sorry, I just saw the last comment now. So ES6 modules don't work in node. Is there no way to ignore them there, or is it a syntax error?

Co-authored-by: Alon Zakai <alonzakai@gmail.com>
@kjlubick
Copy link
Contributor Author

kjlubick commented Dec 4, 2020

AFAIK, there is no way to ignore the export command in node.js

@RReverser
Copy link
Collaborator

Looking at flutter/flutter#58428, it seems that your issue is rather that it's an anonymous define?

I thought these should work fine with Require.js, but, if not, maybe a better fix would be for us to generate some name there?

@RReverser
Copy link
Collaborator

So ES6 modules don't work in node. Is there no way to ignore them there, or is it a syntax error?

Wait, ES6 modules do work in Node nowadays, and few versions ago got even unflagged. Can you try with latest Node.js version and set Emscripten's output extension to .mjs (which implies EXPORT_ES6)?

Base automatically changed from master to main March 8, 2021 23:49
@kjlubick kjlubick closed this Jan 10, 2022
@kjlubick kjlubick deleted the optional-module-code branch January 10, 2022 14: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.

4 participants