Skip to content

Remove --post-js? #6814

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

Closed
kripken opened this issue Jul 5, 2018 · 9 comments
Closed

Remove --post-js? #6814

kripken opened this issue Jul 5, 2018 · 9 comments

Comments

@kripken
Copy link
Member

kripken commented Jul 5, 2018

I think we should remove that option for several reasons:

  • --post-js made a lot of sense for JS and asm.js as startup was synchronous there. So adding some code after the main JS would result in it being run as you expect. Even this became a little complicated with async startup due to things like file preloading, but overall worked well. But with wasm, we assume async startup in the common case. And, if async startup is the norm, then all you need is --pre-js - you add the code you want to run later in a callback, like Module.onRuntimeInitialized. Having --post-js is just an opportunity for confusion.
  • Another possible confusion is that the combination of --pre-js and --post-js has been seen as a way to wrap the entire output in a function scope. This was never intended: we have MODULARIZE for that, and also, wrapping the output in a new scope the optimizer doesn't see confuses it.
  • Removing --post-js will make some future refactorings of our JS easier, in particular Feedback wanted: Allow custom "core JS" replacements, to *really* shrink minimal JS sizes #6803.

This change may require a bunch of work on tests, so I'm opening this issue for discussion first, as opposed to having the discussion on a PR with code.

@Becavalier
Copy link
Contributor

Yes, you are right, actually I usually put all those javascript codes into an anonymous function which will be pushed into the " ATPOSTRUN" structure. The "--post-js" option just used for appending those custom codes to the output javascript file which was generated by "emcc".

@dschuff
Copy link
Member

dschuff commented Aug 23, 2018

+1

@CodeFetch
Copy link

+1 It would be great if you could implement two new options instead "--prepend-js" and "--append-js" which could be used e.g. for initializing modularized code on which one could apply simple minification...

@wojdyr
Copy link
Contributor

wojdyr commented Nov 28, 2019

Another possible confusion is that the combination of --pre-js and --post-js has been seen as a way to wrap the entire output in a function scope. This was never intended: we have MODULARIZE for that, and also, wrapping the output in a new scope the optimizer doesn't see confuses it.

It is used this way in one of the examples:
https://github.com/emscripten-core/emscripten/tree/incoming/third_party/lzma.js

@kripken
Copy link
Member Author

kripken commented Dec 2, 2019

Thanks @wojdyr , looks like that's some very old code we forgot to remove. Removing it in #9942

@prideout
Copy link
Contributor

I love the --post-js feature and would be sad to see it go.

Filament uses --post-js to create Javascript "extensions" to our module, which allows us to add a more JS-friendly API layer than what the embind bindings alone allow for.

For example, our View class takes an options structure that has default values, so in our post-js we have something like this:

    Filament.View.prototype.setBloomOptions = function(overrides) {
        const options = {
            dirtStrength: 0.2,
            strength: 0.10,
            resolution: 360,
            anamorphism: 1.0,
            levels: 6,
            blendMode: Filament.View$BloomOptions$BloomMode.ADD,
            threshold: true,
            enabled: false,
            dirt: null
        };
        Object.assign(options, overrides);
        this._setBloomOptions(options);
    };

This allows us to use value_object for implement the lower-level _setBloomOptions in our C++ bindings, while also allowing clients to skip some fields.

@kripken
Copy link
Member Author

kripken commented May 11, 2020

Meanwhile we have added options similar to what @CodeFetch asked for, --extern-pre-js and --extern-post-js. Those might be suitable for what you want @prideout - specifically --extern-post-js just appends code at the very end, and "externally" to the emscripten compilation process, so it's not optimized or minified etc. (which --post-js is). In your case, does the code need to be optimized/minified together with everything else, or not?

@prideout
Copy link
Contributor

The --extern-post-js option mostly sounds great. It's fine if our extra JS code is not minified along with everything else.

The only issue is that we access some Module internals; specifically, we call GL.registerContext() because we wish to create a WebGL context with specific options. I don't think we can do this from --extern-post-js. Is there a more public mechanism to register a GL context? Our current strategy feels hacky.

@kripken
Copy link
Member Author

kripken commented May 13, 2020

I don't think there is an exported API for GC contexts. So you would need a pre-js or a post-js for it.

I guess that is overall a reasonable reason to keep post-js around, so maybe we should just not remove it. It would also be a little oddly asymmetrical I suppose. I'll close this for now.

@kripken kripken closed this as completed May 13, 2020
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

No branches or pull requests

6 participants