montagejs / mop

Montage Optimizer (mop): Minifies (to reduce file size) and creates bundles (to reduce the number of requests) of Montage applications.
Other
31 stars 15 forks source link

Disable mjson compiler #94

Closed cdebost closed 5 years ago

cdebost commented 5 years ago

A post-17.1.2 commit adds mjson serialization to montage, so that we can do require(“foo.mjson”) and get the instantiated object back instead of the json content. mop has to use montage to load packages in order to have things like the template compiler but in doing so, it sets up the mjson compiler, which will be run when mop picks up .mjson files. Those .mjson files are compiled and executed, leading to code in the constructor of logic modules being executed. This basically means that mop will NOT work on an app that isn’t isomorphic.

An example is a montage app with a montage-data.mjson that eventually loads a service with some logic involving the window or document in its constructor. Since the service will be instantiated by loading the app package, and mop runs under node, the build will error out.

cdebost commented 5 years ago

There is another option to fix this bug, directly in montage. The way we load .mjson files is fundamentally different from how we load JS files (which is how Node loads JS files too).

When requiring JS files, there are two steps. First the file is loaded (read from disk or the network) and a factory is generated for the module. Factories look like this:

module.factory = function (module, exports, require, global, __filename, __dirname) {
    // the actual file contents go here. module, exports, etc. look like global variables
    // to the file, but in reality they are arguments to the factory function
}

This factory is later invoked when a JS file is finally required for the first time. In short, there are two phases to loading a JS file: reading the actual file contents occurs during preload, executing the file occurs when the file is required.

That's not how our MJSON compiler works. When an MJSON file is loaded, both the file loading and execution occur immediately, and the resulting instantiated objects become that module's exports. Execution of the MJSON file is not deferred until a first require is made. That's why we see the constructors of services being executed in mop, which only loads modules.

Imagine a simple JS file:

// foo.js
console.log('Hello, world!')

We wouldn't expect "Hello, world!" to be printed while mop. It in fact will not be printed, because with JS files compilation and execution happen at two different times.

We could change montage to generate a factory function for MJSON files instead of instantiating all of the MJSON's objects in one go. The factory function would probably look something like this:

module.factory = function (module, exports, require, global, __filename, __dirname) {
    var deserializer = require('montage/core/serialization/deserializer/montage-deserializer');
    deserializer.init(module.text, require, void 0, require.location + moduleId); // module.text is the raw file contents
    var root = deserializer.deserializeObject(); // we would need a sync version of this method
    Object.defineProperty(module.exports, 'montageObject', {
        value: root,
        enumerable: false,
        configurable: true,
        writable: true
    });
}
marchant commented 5 years ago

Great proposal, sounds like the way to go

cdebost commented 5 years ago

Merging this despite the failure. Travis is failing because unfortunately one of our transitive dev dependencies upgraded to features that don't work in node 4, and there is no way to backtrack to a version of the dependency that works because we don't have a package-lock.json.