Consider not clobbering System

[guybedford]

Would it be possible to add a conditional check when writing to global.System, and only writing to it only if it does not already exist?

[johnjbarton]

Can you say more about your circumstances?

We install traceur by defining a boot-loading System then we call System.register() to load our own modules and finally load a root module to execute the modules. During the execute phase we overwrite System depending on the platform. What part do you want to take over?

[guybedford]

SystemJS imports Traceur when needed via System.import('traceur'). The result of this call is that the System global is reassigned, which means special wrapper code like https://github.com/systemjs/systemjs/blob/master/lib/extension-es6.js#L95 has to be written to revert the System global.

Happy to provide a PR if it is worth considering.

[johnjbarton]

I prefer to avoid hogging System, but I'm not sure there is a simple fix.

[arv]

I agree that we should fix this.

Can we could conditionally override System if it does not have the methods we need?

[johnjbarton]

I believe the only code that affects @guybedford now is

global.System = {
    register: ModuleStore.register.bind(ModuleStore),
    registerModule: ModuleStore.registerModule.bind(ModuleStore),
    get: ModuleStore.get,
    set: ModuleStore.set,
    normalize: ModuleStore.normalize,
  };

But I also guess that he is not planning to load traceur into SystemJS? That is, he doesn't implement regsiterModule() (and we can't compile to register()). So if we skip the above assignment if System exists, then traceur does not load.

One solution is to change traceur's bootloader name to eg ModuleStore. We would generate code to ModuleStore.registerModule() and ModuleStore.get(). That much is ok since the boot strap is all internal anyway. But then we have to generate different code for bootstrap and regular, working against the self-hosting goal.

If we could compile with instantitate then systemjs would be able to load traceur and we could have ModuleStore not overwrite System.

[guybedford]

Thanks for considering this. If it's possible to change the System references to be locally scoped to Traceur's implementation of System, which may or may not be assigned to the global, then there'd be nothing wrong with self-hosting on System.

Compiling to instantiate would also work though, although may not be preferable as it imposes its naming assumtions in the loader registry.

[guybedford]

I've just hit up against this again as part of separating Traceur loading in SystemJS out into a plugin.

The bootstrap module output of Traceur runtime seems to look like:

(function(global) 
// ...
global.System = { registerModule: ... } 
})(...);
System.registerModule('x', ...)
// ...

I can deal with the overwriting of global.System by reverting the global after execution, but the other problem then is that the System reference is SystemJS instead of the Traceur registration object.

This then makes it impossible for SystemJS to load or build Traceur runtime as a module.

Ideally a bootstrap build that is self-contained should contain self-contained references I think (in the same way SystemJS builds distinguish between bundles and sfx builds).

For example, if the output looked like:

(function(global) 
// ...
global.System = { registerModule: ... } 
})(...)
(function(System) {
System.registerModule('x', ...)
// ...
})(global.System);

then the references would work out fine. Let me know if that makes sense.

[arv]

I'm on a slow path to making the runtime smaller and one thing I want to do is to pull out System from the runtime (or at least make it configurable).

Your refactoring seems like an improvement because then you could change global.System afterwards without affecting Traceur's bootstrap. But on the other hand the bootstrap should only happen once so changing global.System afterwards should not be a problem?

[guybedford]

The specific issue is that when bundling in SystemJS we wrap the code in a System closure on execution, so that the bare System.registerModule will then always be referenced to SystemJS. That's why just ensuring it is properly scoped would fix this with a wrapper like the suggestion above.

I'm open to suggestions though. $traceurRuntime.System.registerModule would work too for example I believe.

Let me know your thoughts and I can look into a PR along these lines.

[guybedford]

@arv just to bump this, is there any path forward here you can suggest? I would really like to be able to provide a Traceur plugin for SystemJS.

[johnjbarton]

We could just replace System with $traceurRuntime. I'll look at that after I get the 'no-modules' version working.

[arv]

@guybedford Supporting SystemJS is high priority for us and we'll make sure things work for you.

I think we should provide a version of Traceur that does not provide its own System or loader since that is not needed when SystemJS is present.

One way to do that is to improve the --modules=inline output of Traceur and make that default for SystemJS builds.

[guybedford]

@arv @johnjbarton sounds great, thank you so much.