Skip to content
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

Fix Electron build #1279

Merged
merged 5 commits into from
Apr 4, 2021
Merged

Fix Electron build #1279

merged 5 commits into from
Apr 4, 2021

Conversation

vardhanapoorv
Copy link
Contributor

Fixes #1273 .

Changes proposed in this pull request:

  • Upgrade electron-builder, electron-devtools-installer & webpack-dev-server packages.

  • Fix Babel config.

@vardhanapoorv vardhanapoorv marked this pull request as ready for review October 20, 2020 18:55
@acao
Copy link
Member

acao commented Dec 16, 2020

thanks so much for working on this @vardhanapoorv ! hoping to finally check this out this weekend and give it a local run through

@vardhanapoorv
Copy link
Contributor Author

That's great!

@acao
Copy link
Member

acao commented Jan 3, 2021

wasn't able to get it running last weekend, hopefully I can get to it tonight!

@vardhanapoorv
Copy link
Contributor Author

@acao Let me know if anymore changes are needed, when you get a chance to review it.

@acao
Copy link
Member

acao commented Apr 3, 2021

@vardhanapoorv nice! it works in terms of fixing the initial schema choice screen

on yarn start, once I supply a remote endpoint though, i get a blank screen and this error
image

@vardhanapoorv
Copy link
Contributor Author

vardhanapoorv commented Apr 4, 2021

@acao I build it using npm run build and then tested release app with npm run release:mac before.
Screenshot 2021-04-04 at 11 29 48 AM

@vardhanapoorv
Copy link
Contributor Author

@acao just tested with yarn start that also seems to be working for me. Let me know if I'm doing something wrong here.
Screenshot 2021-04-04 at 11 35 55 AM

@acao
Copy link
Member

acao commented Apr 4, 2021

so, when I run yarn after cloning your PR, the yarn.lock file is very different. so this might be why its not working here or in CI. If you're able to submit a PR with a clean lockfile then we can move forward! by that i mean, when I run yarn locally after checking out your PR, there should be no change to yarn.lock.

to try and simulate the situation I'm having, try cloning this repo into a new folder, and running yarn, and trying the above commands, and you'll see the lockfile diff and other issues I'm talking about

what that indicates to me is that the node_modules you have locally are working, but whats in the yarn.lock and in the package.json files is not adding up

@vardhanapoorv
Copy link
Contributor Author

vardhanapoorv commented Apr 4, 2021

@acao oh yeah right makes sense, thanks. This reminds me that I was facing some issue with global lockfile so I installed the dependencies for electron package from there which created a separate lockfile in that folder, which I'm thinking is wrong since no file existed before. I'm a bit confused with the build process of the packages in this repo.
After your above comment, I did try a bit but I'm not completely clear if the process I'm following is correct or not. (And yes I face couple of similar errors as you saw).
Can you tell me the steps which should be followed? Should I install dependencies from base folder or from the respective packages folder? Then should I build the packages separately from their respective folders(React build worked, but the html build failed for me)?
Thanks for helping out.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 4, 2021

CLA Signed

The committers are authorized under a signed CLA.

@vardhanapoorv
Copy link
Contributor Author

vardhanapoorv commented Apr 4, 2021

@acao please ignore the above questions, I actually managed to update it correctly I think. Take a look and let me know if you still face any errors.

@acao
Copy link
Member

acao commented Apr 4, 2021

awesome! also, can you sign the CLA for GraphQL Foundation? this is a new thing now that playground is a GraphQL Foundation project, instead of Prisma's CLA bot.

@vardhanapoorv
Copy link
Contributor Author

Done!

@acao
Copy link
Member

acao commented Apr 4, 2021

@vardhanapoorv it works great for me locally! excellent work. not easy to deal with all these package resolutions!

@@ -18,6 +18,7 @@ if (!fs.existsSync(appEntrypoint)) {

module.exports = {
devtool: 'source-map',
mode: 'production',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇

@@ -2,7 +2,7 @@
"compilerOptions": {
"removeComments": true,
"module": "commonjs",
"jsx": "preserve",
"jsx": "react",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇

@vardhanapoorv
Copy link
Contributor Author

@vardhanapoorv it works great for me locally! excellent work. not easy to deal with all these package resolutions!

That's great, thanks for helping me out with my questions throughout this issue!

@acao
Copy link
Member

acao commented Apr 4, 2021

@vardhanapoorv glad to help! the contribution effort is very welcome! if you'd like to help accelerate the follow up, here's what I had in mind for next:

  • upgrade codemirror-graphql
  • upgrade electron
  • add electron build to Github Releases using atlassian changesets and github actions

@vardhanapoorv
Copy link
Contributor Author

Absolutely, sounds good. I can tackle these next. As a separate PR right?

@acao
Copy link
Member

acao commented Apr 4, 2021

@vardhanapoorv yes! separate PR for sure. I will merge this one now. Again, thanks so much!

@acao acao merged commit 9f5f340 into graphql:master Apr 4, 2021
cgxxv pushed a commit to cgxxv/graphql-playground that referenced this pull request Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get Electron build working
2 participants