Support named imports when using node esm loader

[kanongil]

This rewrites the exports for compatibility with node.js commonjs static analysis.

With this PR, Hoek methods and classes can be imported, as expected, when using the ESM loader .

I added a new file to test this. I had to make the import dynamic since Lab does not currently support ESM.

[nlf]

to expand on this a bit, i went digging and found the lexer that parses these and it sounds like the reason the existing code doesn't work is due to the require() statements that are used. an alternative implementation would be to require() each module into a named variable, and then export those named variables.

// this does work
exports.something = require('./something')

// this works
const something = require('./something')

module.exports = {
  something
}

// this does not work, it only exports 'onething' and 'default'
const onething = require('./onething')
const twothing = require('./twothing')

module.exports = {
  onething,
  twothing
}

// this does not
module.exports = {
  something: require('./something')
}

source: https://github.com/guybedford/cjs-module-lexer/tree/1.1.1#exports-object-assignment

this is some top notch bike shedding on my part, but it's worth having the discussion since IMO we should adopt whatever we choose here as our new standard means of exporting and stick with it everywhere.

the change here looks to be the easiest and least likely to allow shooting ourselves in the feet.

[kanongil]

Thanks for the input – good to know that there are other ways to do it. But this way still seems simpler to me.

Any input on the new test? I split it into a new file since it does not relate to the regular module functionality. Ideally, it would be in an esm.mjs test file, but that would require lab support for ESM.

[nlf]

as you mentioned lab would have to support it to put it where it really makes the most sense, so i think this is a really reasonable workaround and a decent convention wherein test/esm.{js,mjs} is where we ensure esm compatibility. will discuss during tsc call today

[dominykas]

As an aside, are the TSC calls/decisions/discussions available publicly? Very much curious to learn more on ESM pitfalls from others, rather than rediscover myself :)

[devinivy]

@dominykas they currently aren't captured into anything that's shared publicly, but it's useful to know that you have some interest. It operates as more of a "standup" and are typically on the order 15-20 minutes, to go over what's been worked on in the past week and what might require attention. The actual work and discussion remains in GitHub issues 👍

It's a bit informal, but I think in this case with ESM, this conversation here is the most it's been talked about anywhere and will probably inform next steps taken on other modules. Nice to see everyone here is in agreement that this is a nice way to deal with ESM exports for now! I agree, this format for named exports plus an ESM test seems to be pretty effective.

When we address any plugins, hapi already supports plugins that are exported alongside other utilities— it assumes the name of the plugin's export is exports.plugin. For example:

// my-plugin.js
'use strict';

exports.utility = () => 'whatever';

exports.plugin = {
    name: 'my-plugin',
    register(server, options) {}
};

// server.js
'use strict';

const Hapi = require('@hapi/hapi');
const MyPlugin = require('./my-plugin');

(async () => {

    const server = Hapi.server();
    await server.register(MyPlugin);
})();