-
-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core.js semver version seems wrong, only considers strict major version #33
Comments
"latest" cannot be the default version because it would cause breaking changes whenever a new minor version of core-js is released (the plugin wouldn't be compatible with the previous version by default). |
breaking would be bad, it's not clear tho why it would break. shouldn't any minor version be compatible with previous ones for the major version? |
Let's suppose that core-js 3.6 doesn't support If I have this code: new Promise().finally(); the generated output might be something like this, which is perfectly compatible with core-js 3.6: import "es.promise";
new Promise().finally(); Now, let's suppose that core-js 3.7 is released. If the plugin defaults to "latest core-js version", then it defaults to a version that supports import "es.promise";
import "es.promise.finally";
new Promise().finally(); However, this code will break if the user had |
ah, right. That isn't great, tho maybe not much of a problem in practice as generally package mgmt sides on the latest compatible semver version. The other side of this is also mildly broken (which is how i got here). where i have the latest corejs installed but babel-polyfill was not polyfiling it. Practically this produces in the same sort of broken runtime time code, tho i'd suggest the later is more likely to happen than the former. in any case, maybe this deserves a more prominent spot in the readme that you must match your local core-js version exactly or miss out on supported polyfills. Could also have the plugin check if there are supported polyfills in a higher version and warn the user? |
I can throw an error if you don't explicitly set the version maybe?
That's a good idea! |
Went to upgrade after #31 was released and noticed that the issue continued to persist. The reason is
get-modules-list-for-target-version
is being fed3
for the version by default, which ignores3.1
,3.2
, etc. I tried a few semver-y things to be more inclusive but it seems core-js doesn't support those, so one needs to manually set3.6
or whatever the latest is, which seems less than ideal.The text was updated successfully, but these errors were encountered: