Andarist / cherry-pick

🍒⛏📦 Build tool to generate proxy directories.
56 stars 12 forks source link

Replace es with more descriptive esm #1

Open TrySound opened 6 years ago

TrySound commented 6 years ago

es may confuse users. It is used for example for distributing untranspiled code in material-ui. esm is consistent with tools. Rollup is gonna rename it too.

Andarist commented 6 years ago

Do you mean the esDir option or it's default (es)? I've chosen this as it relates to how rollup names it and also I've noticed most people using es directory for this so it seemed as reasonable derfault.

Do u have any reference to rollup renaming this?

TrySound commented 6 years ago

I would rely on esm loader here. For rollup deprecating will take some time. https://github.com/rollup/rollup/pull/2102

TrySound commented 6 years ago

I mean all "es" names to "esm" names.

Andarist commented 6 years ago

I'm fine with renaming esDir to esmDir. I'm not sure though if we should rename it's default value. From my experience es is more popular in the wild and it's just a default - it's role is to avoid config for most popular case, so if es is in fact the most popular then we should keep it as default value of that option (I have no actual data to back this statement).

After a thought - we could also try to deduce both esm and cjs dirs from module & main fields. This is only somewhat ambiguous as people might use extension-less values for those and at the time this tool gets used we often might not be able to determine if those entries refer to directories or files.

TrySound commented 6 years ago

I'm trying to convince users to use esm and cjs convention. And they use. As I said there is a precedent in material ui that es is used for untranspiled code.

I think our goal not to follow to the folk but also force them in some cases since we share these ideas.

Andarist commented 6 years ago

Well, there can be found counter examples that would favour keeping es as a default, a single precedent in material ui doesn't quite convince me. Especially that this is configurable and there is always a chance for conflicting names, regardless of what we chose here.

I think our goal not to follow to the folk but also force them in some cases since we share these ideas.

That's a valid argument, although it seems reasonable to aim for ease of use here.

WDYT about detecting those based on module & main?

TrySound commented 6 years ago

Maybe detecting is better

Andarist commented 6 years ago

I've renamed the option (released new version with it), still thinking about default value for it and probably gonna try to implement reliable logic for auto-detection.

TrySound commented 6 years ago

Here's a couple of projects which follow esm convention. And I will continue force it. https://github.com/renatorib/react-powerplug/blob/master/package.json#L8 https://github.com/atlassian/react-beautiful-dnd/blob/master/package.json#L21 https://github.com/souporserious/react-measure/blob/rewrite/package.json#L6 https://github.com/alexreardon/raf-schd/blob/master/package.json#L6 https://github.com/cytoscape/cytoscape.js/blob/unstable/package.json#L70 https://github.com/acdlite/recompose/blob/master/src/packages/recompose/package.json#L13