istanbuljs / append-transform

handle multiple require hooks
MIT License
21 stars 4 forks source link

Better emulate standard `require` for extensions that are not specified: #6

Closed jamestalmage closed 8 years ago

jamestalmage commented 8 years ago

Right now, if you do:

appendTransform(transformFn, '.foo');

And no one ever registers a handler for .foo, we throw an error if a .foo file is ever required.

Without append-tranform what actually happens is that Node just tries to load it as a .js file.

This should be fixed.

Related: #5

// @gotwarlost

jamestalmage commented 8 years ago

As I see it there are two options for when require.extensions['.foo'] === undefined.

  1. We pass through to the .js extension:

    function defaultHook(module, filename) {
     return require.extensions[`.js`](module, filename);
    }

    This would run through the list of extensions for .js, followed by whatever future hooks were installed for .foo.

    Pro:

    • It provides a slightly closer experience to the default require setup if no handler is ever installed.
    • You can just add your append-transform to .js and let Node automatically apply it for .es6, etc.

    Con:

    • It could create weird scenarios where you try to install coverage twice (istanbul installations for .js and .es6 both get executed), etc.
  2. We implement the standard default behavior ourself:

    function defaultHook(module, filename) {
     var content = fs.readFileSync(filename, 'utf8');
     module._compile(stripBom(content), filename);
    }

    Con:

    • You must now install for every extension explicitly (i.e. .es6 in addition to .js, etc).
    • This has implications for extensions outside your control. For example, if Babel were to only install for .js and expect .es6 files to follow along - this would break that.

    Pro:

    • I think it's more straightforward, easier to follow what's happening.

I'm leaning towards 2. My only concern is the Babel example I listed as a con for 2, but I think that would be a bad choice on Babel's part anyways (I'm not actually picking on Babel here - I don't know how they do it - this is all hypothetical).

novemberborn commented 8 years ago

:+1: on 2. This stuff is hard enough as it is, keeping it explicit seems preferable.

jamestalmage commented 8 years ago

I went with 2, deployed as 0.4.0

cc: @gotwarlost