serverless-offline includes a package with an unmet peer dependency

[esetnik]

warning "serverless-offline > [email protected]" has unmet peer dependency "core-js-bundle@^3.1.3".

[dnalborczyk]

hey @esetnik thank you!

it seems jsonpath-plus just added core-js-bundle as peer-dep, but on a quick check they are using it only in a browser test, so it shouldn't be a peer dependency in the first place. would you mind filing an issue over there?

[esetnik]

Ok thank you. I will close this issue once they have resolved the problem.

[brettz9]

@dnalborczyk : We are listing it as a peer dependency on jsonpath-plus because, as a type of plugin, the polyfill may be needed as a result of the babel process. Besides the browser, it might even be needed in some older Node environments: https://babeljs.io/docs/en/babel-polyfill#usage-in-node-browserify-webpack (though those docs are for @babel/polyfill, its replacement--which @babel/polyfill has been using--is core-js). These are just warnings, so if an environment doesn't need it, they can ignore. But I think it is safer to remind users of the possible need to use.

[dnalborczyk]

@brettz9 I don't agree with the use of a peer dependency in this case. people don't need 'reminders' for this kind of thing. you could just plain and simple mention in the README if anyone needs support for IE (fill in your old unsupported version), they need to make sure to load polyfills on their own - that's what everybody does. they might not even choose core-js, as it is EXTREMELY HUUUUGE, and just use individual polyfills. that's also one of the reasons Babel is not including it by default anymore.

Peer dependencies are something which are required to exist, and a plugin couldn't do without. e.g. react for a react-widget, graphql for a graphql plugin, or, in this case, serverless for this plugin (serverless-offline).

[dnalborczyk]

@brettz9 in short: you'll be bugging every single user and project who happens to not use core-js-bundle for one reason or another. which is probably mostly everyone.

[esetnik]

@brettz9 what if it was switched to an optional dependency? It seems like this more closely matches how you are using the dependency.

[dnalborczyk]

@esetnik I'm not even sure it is an optional dependency. remember, there are gazillions of polyfills. this just happens to be one single one of them.

@brettz9 just imagine, if every lib you are using would do such a thing. you'd have endless peer-dependency messages in the console, with all kinds of "suggestions".

[esetnik]

The reality is they should be shipping a fully bundled package containing all necessary polyfills for their support targets. Or they should be shipping an unpolyfilled version with a note in their readme about which polyfills are required. You’re right that creating a dependency on core-js-bundle just leads to unnecessary confusion.

[brettz9]

End of my day here, but plan to review your comments for consideration tomorrow.

[brettz9]

All rightee, you've persuaded me, thanks! Fixed in 0.20.1.

I was considering modifying our @babel/preset-env to set the useBuiltIns option to usage or entry and draw in only those polyfills we're using (so the user wouldn't have to guess at it), but it would still require the whole download.

However, a lot of projects appear to use Babel, so I'd think many could be, or perhaps should be, using core-js (including those still using it through @babel/polyfill), and I'd think it would be more the minority who would be carefully examining which polyfills are actually needed by the package and for their targets and supplying their own non-core-js versions. Maybe Babel could optionally save a config file indicating the polyfills in use so an application wishing to add core-js imports could optionally consume this config information for each of its dependencies and automatically know which imports are needed and add them through a build process to their entry file. (Think I may submit that as a suggestion.)

Also, FWIW, I have seen devDeps use peer deps. in a way where the dep. may not strictly be required, but I guess even if this is not abuse, it won't make as much noise as a regular dependency.

[brettz9]

Also, FWIW, optionalDependencies does not offer command line switches to choose whether to install or not (and they'd still presumably raise a notice if the option were available and not used). Its current purpose is merely to not fail the whole install if the optional one fails. This may be useful for packages which fail because they only work on one platform.

[brettz9]

FWIW, I've filed https://github.com/babel/babel/issues/10087 as a recommendation to allow users to get at the Babel dependencies they need without libraries needing to require them.