Bring back module entry in package.json

[Andarist]

Not sure why this commit has removed it.

jsnext:main and module are the same field (meaning that their goal is the same). jsnext:main was renamed to module and latter is more standard, some tools understands both of them, some need additional configuration to pick up jsnext:main at all and some already even have dropped jsnext:main support.

To sum it up - jsnext:main is older and considered being a legacy field. module is the one that should prevail, although packages might have both to support wider range of tools and their various versions.

[taylorhakes]

Unfortunately that module change caused the library to become unusable for most people. var Promise = require(‘promise-polyfill’); no longer works. Only ES6 modules work. Happy to accept a PR that handles both, but this PR will not be accepted as is. I value backwards compatibility very highly.

[Andarist]

Honestly I don't see why this could have break usage for most people, I've created a sample repository https://github.com/Andarist/promise-polyfill-example which uses this PR as promise-polyfill. If you clone the repo and npm install with npm5 it will nicely install its devDeps and run prepare script, so the lib directory will be created automatically.

You can check it out that I easily use require in my index.js. Node's resolution algorithm just picks up what is inside "main" entry, it doesn't even look into other fields from package.json, so I find it weird that it would break anything. I also couldn't find any issue actually reporting that it breaks things. If there were some, could you point me to them for a reference?

I value backwards compatibility very highly.

Sure thing, me too. I just believe this field was removed based on some wrong assumptions.

[taylorhakes]

module in package.json is used by browserify and webpack. If you have an ES6 file at module, everyone that is not using newer webpack or Babel will have an error. They need to do something like require(‘promise-polyfill’).default which isn’t ideal and not a breaking change specified in version 7.

[taylorhakes]

#67 issue was the cause of the change. Let me know if you have a better way to fix it. Happy to fix it a better way.

[Andarist]

from #67 it is not clear to me what the exact problem was, it certainly doesn't look that "module" was the culprit, because it describes that the issue was happening on IE and "module" would not introduce a broken behaviour only for a subset of browsers

It indeed seems "break" webpack2+ (webpack1 is not module-aware so it wouldnt even try to use this field) WHEN somebody uses require instead of import. There is nothing that can be done to mitigate this on library's side - if one wants to use require in projects built with webpack he/she should add .default, but ultimately require shouldn't be used at all in webpack2+ projects. If one insists to use require over import in such projects he/she is forced to add .default all over the place (when importing any library with module that has default export and there are a lot of them already), so I wouldn't really consider this much of a problem.

I understand that you don't want to break things right now, so I'd advise to just release promise-polyfill@8 with "module" and add appropriate note in the README about need to use .default when using require in webpack2+ (and possibly when using browserify too). This is rather uncommon setup anyway, and certainly does not affect "majority" of users.

You'll do whatever you consider appropriate ofc, but I think this article is relevant https://medium.com/@mikeal/stop-supporting-old-releases-70cfa0e04b0c so I'd encourage you to go with the proposed approach (releasing v8).

[taylorhakes]

I am a little torn on this one. I completely agree that it would be awesome to support ES6 and require together. I don’t agree with the dropping the support of older releases. This library is kind of the exact opposite. It is meant specifically to support older browsers. It is completely useless for newer browsers/node. I prefer keeping the current behavior, unless I get an overwhelming number of people wanting ES6 imports. There isn’t any benefit for defaulting to ES6 currently is there? Tree shaking doesn’t really apply to this library.

[Andarist]

I just want to start with a note that I'm not a native english speaker and while I can express my thoughts, doing it in a polite, pleasant, way is often really hard. I realize that many times my words may sound harsh, but I have no intention of being that way. Always just discussing with an open mind.

Let's go back to the topic though 😄

I completely agree that it would be awesome to support ES6 and require together
Both ways are supported - es6 is working just fine in environments that support it. require is supported too - although ambiguously:

• from cjs-only (i.e. node.js or webpack@1) envs it can be done with require('promise-polyfill')
• in es6-aware tools (i.e. webpack@2+) IF using require it has to be used like this - require('promise-polyfill').default

Sidenote: ambiguity can be eliminated in favor of using require('promise-polyfill').default every time in every cjs context, but cannot be eliminated other way around.

The latter case (using require in modern bundlers) is not that often seen in the wild and should be discouraged, because users of that "style" are losing by using this combination. I've refactored dozens of libraries to es modules and it's the first time I've encountered this problem and this truly leads me to the conclusion that there are not many users of modern bundlers using require calls and even if they do then they are already accustomed to adding .default to bunch of libraries. They also can specify in their webpack config smth like - mainFields: ["main"] to override default mainFields: ["browser", "module", "main"] and be done with "the problem".

This library is kind of the exact opposite. It is meant specifically to support older browsers. It is completely useless for newer browsers/node.

Im all in as to supporting older browsers, problem arises when supporting legacy style (using require) in modern build tooling. This is a different thing.

I prefer keeping the current behavior, unless I get an overwhelming number of people wanting ES6 imports.

Problem with this is that most people dont care, dont notice small things adding up. Only a minority investigates dependencies and tries to improve them for majority of users.

There isn’t any benefit for defaulting to ES6 currently is there? Tree shaking doesn’t really apply to this library.

Tree shaking applies to this and every other library. Even if there is nothing to be tree-shaken off from the library, there is still an issue if library itself is tree-shakeable. I'ce prepared illustration of the problem for other library, but it's applicable to any other. You can read more about it here and see example repository here.

There is also an issue of "scope hoisting" (webpack does this optimization only for es modules libraries) which won't be done if you distribute cjs format only. This is matter of few bytes being added (and runtime startup cost increasing) to consumers' bundles, it is somewhat microoptimization, especially if we consider a single library (promise-polyfill here), but those things add up with each cjs library used in the application and we cannot eliminate those costs without slowly migrating to newer, standard, module format library by library. I've demonstrated the issue here.

If you have any questions regarding mentioned stuff, please let me know so I can explain in more detail.

[taylor-cedar]

I will add this to version 8, along with a couple other breaking changes.

[Andarist]

What I believe should be done to avoid cjs/esm interop problems is to use exports: 'named' in rollup. Otherwise you will end up with this problem #67 if somebody mix up cjs and esm modules within a single app.

[taylorhakes]

Updated version 8 with module reference. Will update with new version of NPM soon. f13a09f

[taylorhakes]

Published version as beta. npm install promise-polyfill@beta