search

iansinnott / react-string-replace

A simple way to safely do string replacement with React components
MIT License
652 stars 56 forks source link

Added ESM version #49

Open niklaswolf opened 4 years ago

niklaswolf commented 4 years ago

See #47 I just added a copy of index.js and changed it to esm. I don't like the code duplication, but I didn't want to introduce a transpiler to output different versions as that would be a big change. If you feel comfortable with doing so, I'd be happy to remove the code duplication.

iansinnott commented 4 years ago

Yeah, the duplication makes me uncomfortable as well because it's the entire file. If we want to change anything at all we'll have to change it in two places and someone might forget, causing inconsistency/bugs.

What are your thoughts on removing the duplication?

We could definitely add a build step and then use use this ESM version as the source fo truth. I like the simplicity of just having JS, but I'm open to bringing in webpack.

niklaswolf commented 4 years ago

I'd love to remove the code duplication, I just did not want to introduce such a big change (add build step) without having your opinion on that. But as you're fine with it, I'm happy to update the PR with a build step using the ES version as source. I don't think we need Webpack at all, since there is nothing to be bundled. Using Babel to transpile the ES version to CommonJS should be sufficiant.

EDIT: damn, forgot about the imports, so bundler is needed, you're right

niklaswolf commented 4 years ago

What do you think about the export format? I don't see any value in making a commonJS export, as this will never be run on node alone. A export for IIFE (browsers) seems more suitable. Besides that, every bundled React application will use the ES version I think, so the only value for having another export format is when people import this directly via a script tag. What do you think?

iansinnott commented 4 years ago

Maybe you're right about just using babel. This is meant for react apps so whoever is importing this lib will probably have some kind of build pipeline set up. If we can transpile ES code to the form it is now (CommonJS) then that should allow moving forward without breaking anyone's build, right?