This repository was archived by the owner on Nov 4, 2024. It is now read-only.
Repository navigation
This repository was archived by the owner on Nov 4, 2024. It is now read-only.
[Question] Why does import require functions on the prototype to be bound to the instance? #797
Description
I'm working on some sample code for gulp and found that I can import series or parallel successfully but not src or dest.
Examples:
// gulpfile.esm.js
import { series, parallel } from 'gulp';// gulpfile.esm.js
import { src, dest } from 'gulp';
// errors with SyntaxError: The requested module 'file:///User/home/v4/node_modules/gulp/index.js' does not provide an export named 'src'
// errors with SyntaxError: The requested module 'file:///User/home/v4/node_modules/gulp/index.js' does not provide an export named 'dest'I've found that is because gulp does the following:
function Gulp() {
Undertaker.call(this);
// Bind the functions for destructuring
this.watch = this.watch.bind(this);
this.task = this.task.bind(this);
this.series = this.series.bind(this);
this.parallel = this.parallel.bind(this);
this.registry = this.registry.bind(this);
this.tree = this.tree.bind(this);
this.lastRun = this.lastRun.bind(this);
}
util.inherits(Gulp, Undertaker);
Gulp.prototype.src = vfs.src;
Gulp.prototype.dest = vfs.dest;
Gulp.prototype.symlink = vfs.symlink;
module.exports = new Gulp();If the following lines are added to the constructor, the imports work successfully:
this.src = this.src.bind(this);
this.dest = this.dest.bind(this);
this.symlink = this.symlink.bind(this);I'll likely be patching gulp so everything works correctly with esm but I'm just wondering why this issue has occurred.
Activity
jdalton commentedon May 4, 2019
DUUUDE @phated!
There was a regression in the last release related to detection of handling of CJS exports. A combo of things have slowed down pushing an update (vacation, switching jobs, life stuff). I'm hoping to get back into the rhythm of things soon and cleanup the remaining
esmissues. If you could create a small repro repo or a gist for me to test I could let you know if ourmasterbranch fixes the issue.phated commentedon May 4, 2019
Congrats on the new job.
Here's a minimal repro: https://github.com/phated/gulp-esm-issue - should be able to replicate with
npm install && npm testjdalton commentedon May 6, 2019
Ah okay!
On review of the repro the issue is by design as ESM (the module system not the
esmpackage) needs a way to compute its namespace object. We currently do this by getting the own properties of CJS exports. This is why binding to thethisworks, since they become own properties of theGulpinstance. I could try to maximally flatten the object but that makes for a potentially messy namespace object (all inherited properties assigned as own properties).phated commentedon May 6, 2019
Cool, I thought it might be something like that. I'll be cutting a release that implements the fix on gulp's end. Thanks
Fix: Bind src/dest/symlink to the gulp instance to support esm export…