search

hapijs / hoek

Node utilities shared among the extended hapi universe
Other
481 stars 171 forks source link

Explicitly export modules #380

Closed kanongil closed 1 year ago

kanongil commented 2 years ago

This patch uses the package.json subpath exports and conditional exports features to explicitly export all embedded tools.

This allows consumers to require / import only the tools that they need. This is already possible, so this primarily makes it formal, a bit better looking, and supported by test cases.

The breaking change comes from the use of subpath exports. Where you would previously require eg. "assert" as require('@hapi/hoek/lib/assert'), it is now only exposed as require('@hapi/hoek/assert'). For import it would have been import assert from '@hapi/hoek/lib/assert.js' which is now import assert from '@hapi/hoek/assert'. Additionally, it is no longer case insensitive. It also means that it is no longer possible to require "private" helper files, like lib/utils.js or lib/types.js.

As part of this I made an index.mjs file that is a module version of the index.js file. This is used as the new primary entry-point when loaded as a module. Due to lab not supporting modules for coverage, it is not part of the coverage check, but given that there are no branches, this should be safe enough.

Note, I have not exposed the exported entry points in the typescript definitions. This can be done in a future non-breaking patch, if it makes sense.

kanongil commented 2 years ago

FYI, this is not something I see adopted across the hapijs ecosystem, but I think it makes sense for hoek which is a grab bag of functionality.

Nargonath commented 2 years ago

Yes sure I agree. hoek is the lodash of the hapi ecosystem and is pretty much framework agnostic so makes sense to ship it that way.

Marsup commented 2 years ago

Wholeheartedly agree with this PR, maybe consider another one (#381) which I just opened before release.

Marsup commented 1 year ago

Apologies @kanongil, I forgot this PR in the 11 release. I have tried to rebase this PR due to conflicts, but then the tests started to fail. I made several attempts to try and fix them, but I think the simplest way is not to have an error file containing multiple exports, but stick to 1 file = 1 default export as it is the norm in this module. Let me know if this betrays your original idea, and if you agree with the changes I made.