Esmification

[aerialist7]

Hi Team, I've prepare the ESM version of the library. Can you please review the changes and merge them if OK?

[aerialist7]

Hi @P0lip, hi @philsturgeon

I've fixed the ESLint, but seems tests will fail because of type: "module".

Can you consult me, how to correctly update project config to enable es-modules for test environment? P.S. Here you can find a draft of tests esmification - https://github.com/aerialist7/json-schema-ref-parser/tree/esm-tests

[philsturgeon]

@aerialist7 I really appreciate all the work, but I've had to ask some folks opinions about this change as it's rather huge, and I'm not as familiar with ESM vs CommonJS as I'd like to be. I mostly maintain this package until new contributors and/or maintainers can take over.

I wouldn't go all-in on ESM like this yet. Lots of people still use CommonJS in node, and switching to ESM-only like this makes modules very awkward to use for them. It's definitely a breaking change, and a very inconvenient one in some cases. To keep using the package, everybody downstream would have to either switch to ESM too, or start using async import() + promise calls to import the module.

Node-fetch did it, and it was not popular: https://github.com/node-fetch/node-fetch/issues/1263

ESM is absolutely the future, but it's still in an awkward phase for now. It is possible to support both at the same time via the exports field I hear, if you compile the project into both ESM & CommonJS forms, but it'd require some setup. Personally for existing projects like this, I wouldn't bother until you have a good reason to actively switch over.

That is of course just one opinion, but the node-fetch issue is fairly concerning by itself.

As such im wonering if this is something that needs to be done, or if it would be better off done in the stalled replacement package which can handle loads more than this one, but isn't ready yet.

https://github.com/APIDevTools/json-schema-reader

Perhaps we could make this change over there, and if you're up for it you could be one of the team that gets this package over the finishing line? It'll do a lot more good for $ref and $id (new OpenAPI 3.1 stuff).

[philsturgeon]

@aerialist7 I understand this situation a lot better now, and I see that there's a way to go ahead with this pull request if you're still interested!

https://dev.to/bcoe/esm-doesn-t-need-to-break-the-ecosystem-4p8b

They talk about conditional imports:

  {
    "exports": {
      ".": {
        "import": "./index.mjs",
        "require": "./index.cjs"
      },
      "./helpers": {
        "import": "./helpers/helpers.mjs",
        "require": "./helpers/index.js"
      }
    }
  }

Can we set that up so things will continue to work for CJS and ESM?

Then we'll avoid the problems discussed in the node-fetch issue.

[aerialist7]

Hi @philsturgeon, thank you for your respond. I think we can, but we need to configure build to CJS and ESM together.

[philsturgeon]

I'll admit I'm not caught up with all this, but I have blundered my way through it with help on other projects.

https://github.com/stoplightio/spectral-owasp-ruleset/blob/main/package.json

We got that repo building from TS to CJS and ESM so perhaps something can be nicked from there.

Do you think you could take a swing at it?

[philsturgeon]

I've just merged two large changes which will have caused some conflicts here, sorry about that. I'm done merging big changes now though so you've got a clear runway to land this change.

[deepakthankachan]

any updates on this? cheers

[philsturgeon]

@deepakthankachan you know if this PR has updates because it will show them. Currently there are conflicts and failing tests so we're just waiting for that to get fixed.

[wparad]

The best way to proceed with this is first add in building the library with a tool that can output both esm and cjs and then do the esm changes if we want. There shouldn't really be a reason we need to introduce esm though, since esm can already import cjs files which the library currently is. If we see the value in converting to esm in the long run then making those changes piecemeal in follow up PRs is the best way to go.

[philsturgeon]

@acunniffe any chance you can pick this PR up and get it over the line?

[philsturgeon]

I've made progress over here if anyone is interested in continuing this effort. #288

[philsturgeon]

More progress is being made in #288 and this one isn't being worked on so I'll close this for that.