0.50: change to new globals #494
Conversation
| @@ -107,7 +107,7 @@ | |||
| return moduleRecords[key] || (moduleRecords[key] = { | |||
| key: key, | |||
| dependencies: [], | |||
| module: new Module({}), | |||
| module: new Reflect.Module({}), | |||
There was a problem hiding this comment.
I don't think the above matter too much? Module is an internally scoped reference to Reflect.Module and not global.Module I think?
There was a problem hiding this comment.
See https://github.com/ModuleLoader/es6-module-loader/blob/0.50/src/loader.js#L682, we rely on internal scoping through the build process.
It may well be worth rewriting the build using ES modules and Rollup as another task here to make these scoping assumptions clearer!
There was a problem hiding this comment.
I would prefer having these add-ons as modules :-) Module() is surely left over from the old spec, and I think using Reflect.Module makes it clearer what's going on here, though I agree it probably doesn't matter much in practice.
There was a problem hiding this comment.
It may well be worth rewriting the build using ES modules and Rollup as another task here to make these scoping assumptions clearer!
just to make sure we're on the same wavelength, you're proposing rewriting src/*.js as ES6, and then use Rollup as build instead of concat to convert that into IIFE? I think that would be a very good idea - and would suggest extending to SystemJS as well.
|
Module() is marked as TODO, so we can leave that till later. What is System.global used for? It's not in the spec AFAICS. |
|
System.global is a separate spec at https://github.com/tc39/proposal-global. |
No description provided.