search

FormidableLabs / react-fast-compare

fastest deep equal comparison for React
MIT License
1.59k stars 54 forks source link

Add esm support #40

Closed TrySound closed 5 years ago

TrySound commented 5 years ago

In this diff I converted the lib to esm and bundled it to cjs and esm with rollup.

Modern bundlers will consume module by default.

It may be considered as minor change.

ryan-roemer commented 5 years ago

Thanks for thinking of us!

All of our OSS that has multiple files, particularly multiple re-exports has both ESM and CJS output. However, here we have one file that we need all of. And all bundlers easily consume CJS via package.json:main.

So, offhand, I don't think we need to add ESM options, especially given that we don't have a build + release step right now and this adds one

Separately, if we do want to take this PR, a few things:

  1. we have package.json:files here https://github.com/FormidableLabs/react-fast-compare/blob/master/package.json#L75-L78 which would need to be updated as none of the new build in dist/ is actually published...
  2. The TS defs need to be published too. And I think they might be broken now (CI is failing for ts checks).
  3. if all we need is ESM-to-CJS conversion, might be easier/lighter to do it with babel instead of rollup. We have just one file, so rollup seems a bit overkill

/cc @chrisbolin

TrySound commented 5 years ago

There's a difference in babel and rollup output. Babel always produces exports.default + exports.__esModule fields. They need to be handled somehow. Rollup produces the same output as this lib had before module.exports = fn. So I'd say rollup fit perfectly to this build step. It's fast, doesn't have dozens dependencies. Babel is overkill with its configuration.

I don't have an idea what TS wants from me. Need some help with this.

ryan-roemer commented 5 years ago

side note -- outside of TS failures, all the tests will fail because we require index

TrySound commented 5 years ago

Well, TS blocks tests

chrisbolin commented 5 years ago

@TrySound thanks so much for the contribution and getting into the weeds with us 😄

I think I'm still not sold, as this is a one-file library with no dependencies. I would also like to stay very close to fast-deep-equal, as we're a fork of that.

I think it all boils down to this: What optimizations would library consumers see with cjs and esm output?

chrisbolin commented 5 years ago

closing for lack of activity. feel free to comment and we can open it back up!