Use module.watch(require(id), ...) to avoid calling module.require.#159
Use module.watch(require(id), ...) to avoid calling module.require.#159
Conversation
|
A really interesting question is whether a package that uses a version of Reify that generates |
lib/runtime.js
Outdated
| if (utils.isObject(setters)) { | ||
| Entry.getOrCreate(exported).addSetters(this, setters, key); | ||
| } | ||
| return moduleWatch.call(this, this.require(id), setters, key); |
There was a problem hiding this comment.
this.watch(this.require(id), setters, key)
|
Would this deprecate |
|
Heads up: I rebased this PR. After this is merged it may be a good time to revisit the problem of referencing |
Yes, I think |
de18e05 to
ce6bd37
Compare
|
Cool. We could deprecate it in v0.11 and remove it in v0.12 (that's being super nice) |
|
Do you have any preferred techniques for giving not-too-noisy deprecation warnings? |
|
I'd put it in the readme for v0.11 and you can add a note on npm install with npm deprecate just for v0.11. |
lib/import-export-visitor.js
Outdated
| pad(this, "module.importSync(", decl.start, decl.source.start) + | ||
| getSourceString(this, decl) + | ||
| pad(this, "module.watch(", decl.start, decl.source.start) + | ||
| "require(" + getSourceString(this, decl) + ")" + |
There was a problem hiding this comment.
Should the "require(" + getSourceString(this, decl) + ")" bit be in either the first pad() or the following one?
There was a problem hiding this comment.
Good catch, I think it should go in the one above, since it's usually (always?) only one line, and that line is the first line of the module.watch expression.
I definitely agree we should do that for |
If we choose to go in this direction, we would be removing a significant barrier to interop with CommonJS module systems other than Node (in particular, Webpack and Browserify), since module.require would no longer be a requirement, and third-party bundling tools could easily understand dependencies embedded in module.watch(require(id), ...) calls. I'm not committed to module.watch as a method name, though it's short and accurately reflects that the parent module is watching an exports object returned by a child module.
|
If we choose to go in this direction, we would be removing a significant barrier to interop with CommonJS module systems other than Node (in particular, Webpack and Browserify), since
module.requirewould no longer be a requirement, and third-party bundling tools could easily understand dependencies embedded inmodule.watch(require(id), ...)calls.I'm not committed to module.watch as a method name, though it's short and accurately reflects that the parent module is watching an exports object returned by a child module.