Add package.json "exports" field

[flevi29]

Pull Request

I am aiming to slowly try to update and simplify this repository, and maybe learn a thing or two along the way.

What does this PR do?

• add "exports" field to package.json
  • It is the new way of defining your exported files, instead of just relying on one single "main" export for Node.js
  • This way Node.js can conditionally determine which exported file to import based on the fact that require or import was used, previously Node.js even in ESM mode always imported the commonjs version of this package, but now it can import the ESM version
  • https://webpack.js.org/guides/package-exports/
  • subpath-exports
  • conditional-exports
• add node: specifiers to Node.js built-in module imports/requires
• add missing @types/node type definitions development dependency
• because older versions of jest test runner doesn't support node: specifiers, update jest (Update Jest #1622)
• because newer versions of jest doesn't include anymore "jest-environment-jsdom", add it as development dependency
• because newer versions of jest simplified snapshots, update them accordingly
  • this means replacing Object { with { and Array [ with [

PR checklist

Please check if your PR fulfills the following requirements:

• Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
• Have you read the contributing guidelines?
• Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

[curquiza]

Hello @flevi29
Thank for your multiple PRs 🙏

Do you know why we have a failure on Node 18?

[flevi29]

Somehow expect.any(Array) checks if the provided value is instance of the global Array, but I'm not sure how and what happens along the way that this starts failing starting with Node.js 18. All I really know is that's not how you're supposed to check if a value is an array or not in JS. It's detailed here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/isArray#description

I might want to open a separate PR for updating Jest instead of doing it here.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.43%. Comparing base (7b2054d) to head (5f7b999).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1611   +/-   ##
=======================================
  Coverage   97.43%   97.43%           
=======================================
  Files          22       22           
  Lines         858      858           
  Branches       86       86           
=======================================
  Hits          836      836           
  Misses         21       21           
  Partials        1        1           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[curquiza]

I might want to open a separate PR for updating Jest instead of doing it here.

As you wish. If the jest update is quick (does not impact a lot of files), you can do it here
Indeed, if the jest update requires modifying all our tests, then, another PR is better

[flevi29]

Waiting on #1621

[curquiza]

Hello @flevi29

#1621 has been merged

you can rebase 😊

[flevi29]

The thing about this new exports field is that everything not included in it becomes unreachable. I can see that src is included in the final npm package, I assume it is for easier debugging with the help of source declaration maps.
I'm not sure if it was meant for people to actually transpile/bundle themselves, considering that in rollup config file there's a line specifically saying that the ESM bundle is for bundlers. If so in module resolution Node16 it seems it would be unreachable (to TS anyways, so no type checking at the most), as in that mode TS respects exports.

In that case we'd need

"exports": {
    ".": {
      // ...
    },
    "./src/*": "./src/*.ts"
  },

But is this something we'd want? We had issues with people accidentally importing the source files, but I'm not sure if people so regularly use src directly otherwise, or if they should at all.

[flevi29]

I finally might've rebased properly, changes are nice short and readable. I'm still learning git.

[brunoocasali]

Hi @flevi29, how are you?!

Do you have a Discord user or a mail address to share with me? I want to discuss your contributions to our repositories in more detail, and I have a proposal that should give you more freedom to contribute.

If you're interested, let me know!

[brunoocasali]

Hi @flevi29, are you interested in continuing to work on these improvements?

[flevi29]

Hi @brunoocasali , sorry for not getting back to you. I might be interested in the future to work more on this, but it's just quite a bit of work and at the moment I don't really feel all that motivated.
I hoped that I'd get feedback on these PRs, but as it stands you have no one at Meilisearch who really understands these things or has time for reviewing these PRs or both.

[brunoocasali]

Hi @brunoocasali , sorry for not getting back to you. I might be interested in the future to work more on this, but it's just quite a bit of work and at the moment I don't really feel all that motivated. I hoped that I'd get feedback on these PRs, but as it stands you have no one at Meilisearch who really understands these things or has time for reviewing these PRs or both.

I see what you mean and I'm sorry for not being able to follow your work as close as I would like.

In the past few months, we onboarded some contributors to assume the management of our libraries like ruby/rails, dart, php/symfony and python. We gave them full creative control over the source code and help when needed, which has worked nicely ever since.
We had to do that to focus on other aspects of the product because we are a compact team.

If you're interested, we could set that up with you here in the main js repos if that would fit you no pressure.

In any case, your contributions are always welcomed 🙏 .

[flevi29]

The thing about this new exports field is that everything not included in it becomes unreachable. I can see that src is included in the final npm package, I assume it is for easier debugging with the help of source declaration maps. I'm not sure if it was meant for people to actually transpile/bundle themselves, considering that in rollup config file there's a line specifically saying that the ESM bundle is for bundlers. If so in module resolution Node16 it seems it would be unreachable (to TS anyways, so no type checking at the most), as in that mode TS respects exports.

In that case we'd need

"exports": {
    ".": {
      // ...
    },
    "./src/*": "./src/*.ts"
  },

But is this something we'd want? We had issues with people accidentally importing the source files, but I'm not sure if people so regularly use src directly otherwise, or if they should at all.

I decided against including src. If you think otherwise, let me know.

[flevi29]

Actually this could be a breaking change for anyone who was importing anything other than what we have in the "exports" package.json field.