[Question] Why does import require functions on the prototype to be bound to the instance?

[phated]

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.

[jdalton]

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 esm issues. If you could create a small repro repo or a gist for me to test I could let you know if our master branch fixes the issue.

[phated]

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 test

[jdalton]

Ah okay!

On review of the repro the issue is by design as ESM (the module system not the esm package) 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 the this works, since they become own properties of the Gulp instance. 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]

Cool, I thought it might be something like that. I'll be cutting a release that implements the fix on gulp's end. Thanks