Fill data with electron as a target. #229
Conversation
Codecov Report
@@ Coverage Diff @@
## master #229 +/- ##
=========================================
- Coverage 93.06% 92.7% -0.37%
=========================================
Files 4 4
Lines 202 192 -10
Branches 60 57 -3
=========================================
- Hits 188 178 -10
Misses 9 9
Partials 5 5
Continue to review full report at Codecov.
|
scripts/build-data.js
Outdated
| const chromiumVersionsLength = chromiumToElectronVersions.length; | ||
| const maxChromium = +chromiumToElectronVersions[chromiumVersionsLength - 1]; | ||
| if (targetVersion > maxChromium) return null; | ||
| const closestChrome = chromiumToElectronVersions.reduce((result, version) => { |
There was a problem hiding this comment.
This results an incorrect version of electron being mapped, for example for async:
{
"chrome": 56,
"electron": 1.5
}Electron should be 1.6.
Should be able to reduce (hehe) the reduce with something like:
const closestChrome = chromiumToElectronVersions.find((version) => targetVersion <= version);EDIT: even better, I guess we can drop findClosestElectronVersion completely and use chromiumToElectron once it lands?
/cc: @Kilian
There was a problem hiding this comment.
@existentialism so we need to update chromiumToElectron. for now, it will just return undefined if no explicit values were found.
|
1.3.0 is released! Thanks @yavorsky :) https://www.npmjs.com/package/electron-to-chromium |
|
@Kilian there's a regression with 1.3.0 |
|
v1.3.1 up now, sorry about that. |
|
@Kilian please, join BabelJS slack channel |
|
FYI, this broke Electron Forge, because we assume that we can send a full Electron version (as a string) as a target version. Downgrading from 1.3.0 to 1.2.2 fixes it. I need to see if #231 fixes this for us. |
|
Electron version value was inconsistent: all targets must be a number, but @malept Is it a problem to use numeric float value? Update: electron/forge#187 will fix issue. |
|
@yavorsky Might have been inconsistent but this change should have been considered breaking and been bumped as such IMO 👍 |
|
@MarshallOfSound It was rather bug fix than breaking change 🙂. According to the docs, target must be only a number (unless we merge #231). But yeah we should add this at least to the changelog. |
|
broke me too. I realize that the "correct" handling of this is a bit vague, but generally it'd be good side on the one that doesn't break folks, even if it was technically bug. Not a huge deal but just adding my voice. Thanks ya'll |
|
@MarshallOfSound @jquense sorry for the unintentional breakage, we restored string version support for electron in https://github.com/babel/babel-preset-env/releases/tag/v1.3.3! |
For now, we are converting electron version to chrome with
electronToChromiumwhen building the plugins list because ofcompat-tablehas no info aboutelectron.With this PR we could convert all chrome version we need to support into electron like we always do for the
opera.It will make consistent log output, better
preset-envdebugging, performance optimization (less work while building plugins for the preset). Alsodata/pluginsordata/built-insis a solid universal source that could be useful for different things in the future, so filling it is a good idea as for me.