decor/schedule should be loadable during the build.

[clmath]

This issue is the follow up of ibm-js/delite#402

@asudoh said:

Guessing @clmath is using node.js 0.8, where setImmediate() is not available. Create a PR adding process.nextTick() support to decor/schedule to cope with that. Also - setImmediate() runs after painting, etc. stuffs happens, whereas end-of-microtask queue like process.nextTick() and Mutation Observer callbacks runs before painting, etc. stuffs - Which means process.nextTick() is much closer to Object.observe() spec than setImmediate() is.

Actually I am using node v0.10.33. I have checked and setImmediate() is available.
The problem is that the has plugin is set to not execute feature tests during the build since they could lead to incorrect modules inclusion in the layer.
So has("setImmediate") is always false during the build even on node v0.10.33.

So maybe a quick fix can be to have a typeof window !== "undefined" check before accessing window.

[asudoh]

Thanks @clmath for elaborating what the problem is.

So has("setImmediate”) is always false during the build even on node v0.10.33.

This leads to a bigger problem than access to window object. If has("setImmediate”) as well as has(“mutation-observer-api”) are false the code tries to emit and receive Web Messaging Event, which is does not make sense for node.js (thus build) environment.

Given feature detection and branching upon its result is one of the key use cases for has.js, I’d suggest fixing the build system therefore.

[asudoh]

Updated my change introducing process.nextTick() support to check for window object existence, which may help non-browser, non-node.js environment. Still suggesting to fix the build system.

[wkeese]

I don't think we should bloat the code with support for non-browser non-Node (or old Node) environments. No one will ever use that code branch.

IIUC we can work around this weirdness with the build system by simply changing:

} else if (!has("setimmediate-api")) {

to

} else if (!has("setimmediate-api") && typeof window !== "undefined") {

(like you did at asudoh@9d92fe4#diff-13ae235e4b68af8228369014f462fd2bR50)

[wkeese]

PS: About the build system, I agree it's confusing to not execute has.add() blocks at all, but OTOH builds are for browsers, so it would be silly to make a build based on feature-tests for what Node.js supports.

[asudoh]

I thought the build system loads code in two different ways? One is for inclusion in layers where no code runs and has() check should be based on running browser’s environment, another is for building plugins where has() check should be based on the build’s node.js environment?

[asudoh]

BTW basically I don’t think it would make sense to introduce a workaround specifically for our specific build system.

[wkeese]

IIUC, as you said, the build system needs to execute the code in plugins, specifically writeLayer(). But, in order to execute the code in the delite/handlebars! plugin, it needs to load delite/handlebar's direct and indirect dependencies, including decor/schedule.

BTW basically I don’t think it would make sense to introduce a workaround specifically for our specific build system.

I don't know any other way to solve the problem. FWIW, I did think of many other workarounds though.
The issue is this code block, which runs even if no one ever calls schedule():

if (has("mutation-observer-api")) {
    pseudoDiv.id = 0;
    new MutationObserver(runCallbacks).observe(pseudoDiv, {attributes: true});
} else if (!has("setimmediate-api")) {
    window.addEventListener("message", function (event) {
        if (event.data === uniqueId) {
            runCallbacks();
        }
    });
}

So some other workarounds are:

1. Change if (!has("setimmediate-api")) to if (has("host-browser") && !has("setimmediate-api"))
2. The code above could be surrounded by if (!has("builder")) { ... }. (That's the one has() flag that is set during builds.)
3. You could defer running the code above until the first call to schedule(). Probably not a good idea, since it introduces a check on every call to schedule().

[asudoh]

My point was that the environment running writeLayer() should have appropriate has() flags for node.js environment for build system (e.g. true for has(“setimmediate-api”)). @clmath have you tried mapping modules features.js depends on to an empty module, in case you didn’t want those to be included in the layer? (Otherwise I need to have better understanding on your concern)

In this context, if we set false for has(“host-browser”) for the node.js environment for build that’s better than nothing as we can go for 1., which is better aligned to overall structure of schedule.js, using has() for feature-based branching.

[cjolif]

Can't we be pragmatic? We have very little time for that right now. We have a one word workaround for schedule.js and we would have a day of work to adapt the build system. So what about for this release implementing the workaround and registering an issue on requirejs-dplugins/has so that later we can implement the proper solution?

[wkeese]

If it's only a day of work to fix the build system then I would go for that.

[cjolif]

@wkeese If you have it, feel free to do it, but we don't :(

[asudoh]

@wkeese @clmath Just to double-check, my interpretation of #45 (comment) is has(“host-browser”) returns the right thing (false) for the build system, is it correct?

[wkeese]

@asudoh - that's my understanding too but I've never run the builder.

@cjolif - I have no interest in maintaining the builder code. IMO we shouldn't have our own builder at all. Especially if you don't have time to maintain it.

[asudoh]

+1 for not maintaining the builder by our own. I heard it’s for some of our plugins, but I think we should minimize what our builder code does at least. Probably amodro-trace would help on that.

[clmath]

has("host-browser") will indeed be false during the build.

[asudoh]

Thanks @clmath for your feedback, let's go for 1. in #45 (comment) for now then.

[clmath]

I created the issue against requirejs-dplugins/has. ibm-js/requirejs-dplugins#22