Update Electron to v6

[razfriman]

Update to Electron 6.0.0.

Update packages:

• electron
• electron-builder
• electron-rebuild

Update the nodeIntegration default which was changed in Electron 5.0.0.

Add missing plist package required from updating the packages

[razfriman]

Anyone able to help me figure out why the CI is failing. I don't seem to be getting all of these warnings and errors when I build the app locally

[Stanzilla]

You need to update node-pty. the old version does not work with node 10+ afaik

[Stanzilla]

Hey @razfriman are you willing to pick this up again?

[Stanzilla]

@Tyriar any idea why this fails? https://travis-ci.org/zeit/hyper/builds/583470356?utm_source=github_status&utm_medium=notification

[razfriman]

I seem to be having issues doing this upgrade actually. Things don't seem to be working well for me with node 12 and electron 6. Perhaps someone else can take this over

[Tyriar]

@Stanzilla this is the problem, you need to get on one of the newer beta builds for Electron 6 support. Try node-pty@0.9.0-beta26

[Stanzilla]

@Tyriar hrm, still errors :(

[Tyriar]

@Stanzilla You might need to also tweak the version of node you're building with. @deepak1556 what version of node.js should builds be using with Electron 6?

[Stanzilla]

C:\projects\hyper\app\node_modules\node-pty>if not defined npm_config_node_gyp (node "C:\Program Files\nodejs\node_modules\npm\bin\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" rebuild )  else (node "" rebuild ) 
gyp info it worked if it ends with ok
gyp info using node-gyp@3.8.0

it seems to be using an old version of node-gyp, not sure why

[Stanzilla]

@Stanzilla You might need to also tweak the version of node you're building with. @deepak1556 what version of node.js should builds be using with Electron 6?

https://electronjs.org/blog/electron-6-0 says 12.4.0, CI seems to use gyp info using node@12.6.0 | win32 | x64 so should be fine?

[deepak1556]

@Stanzilla electron@6.0.8 will break native module compilation on windows due to electron/electron#20185, it has been fixed yesterday and new version will be released today, please update to that when its available. It should fix the build issues you are seeing.

[Stanzilla]

@deepak1556 oh interesting, thanks! I hope this really is it then, because I tried with 6.0.0 before and it was the same issue

[ivanwonder]

oh, missed it. wait for the feedback of @LabhanshAgrawal about the initial load resize issue.

[Stanzilla]

yup!

[LabhanshAgrawal]

The initial load resize is working. I can see it being rendered a bit small and then resizing but it's very quick and isn't really a problem according to me.
The text rendering thing is still there though.

[Stanzilla]

@LabhanshAgrawal can you make a screenshot/video of that remaining problem, please?

[ivanwonder]

I think the text rendering is the problem of fonts or browser. https://stackblitz.com/edit/typescript-dausj9?file=index.ts you can test with different fonts.

[LabhanshAgrawal]

@Stanzilla

[Stanzilla]

Oh yeah, I'd probably blame that on fonts

[LabhanshAgrawal]

@ivanwonder in this I haven't changed anything, the text looks moved up to me. Is this supposed to be like this?

[LabhanshAgrawal]

Ok I tried with my terminal fonts. Maybe the issue is really with the fonts. If you're not facing any problem then you should ignore this as it might be an issue with my patched fonts.

[ivanwonder]

Look the alphabet j, it's in the middle.

[LabhanshAgrawal]

yep, seems like my powerline glyphs are buggy

[ivanwonder]

if you mind this, try render type dom. it shows ok.

[LabhanshAgrawal]

it's ok, I'll look for new fonts

[ivanwonder]

if #3848 has been solved. maybe close.

[Stanzilla]

I will once we merge this one.

[LabhanshAgrawal]

@ivanwonder I tried out DejaVu Sans Mono for Powerline still have that gap at bottom.
I don't have any issue with 3.0.2 so it shouldn't be a font issue .

[Stanzilla]

@LabhanshAgrawal but that was before the electron update, caused by the xterm update, right? so not blocking this PR.

[LabhanshAgrawal]

Checked with canary, no font issue there also

[Stanzilla]

That does not answer my question though :D

[LabhanshAgrawal]

I noticed it in xterm update, but it was working on using canvas renderer. But it's not working with this PR even with canvas renderer.

[Stanzilla]

ok

[LabhanshAgrawal]

But I think the issue is small, so it should not block the PR, we can track it separately.

[Stanzilla]

yeah, agree

[LabhanshAgrawal]

there's nothing else blocking this, right?

[Stanzilla]

nope I just some last testing and everything looks good, so, merging!

[ivanwonder]

because there are different version of chrome between them , run that page on hyper 3.0.2

…

________________________________ 发件人: Benjamin Staneck <notifications@github.com> 发送时间: Friday, October 11, 2019 3:20:47 AM 收件人: zeit/hyper <hyper@noreply.github.com> 抄送: ivan <ivanwonder@outlook.com>; Mention <mention@noreply.github.com> 主题: Re: [zeit/hyper] Update Electron to v6 (#3785) Merged #3785<#3785> into canary. ― You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#3785>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADYVH3HDG7BZ536CNNESUYTQN56A7ANCNFSM4IJZQPHQ>.