-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Added Integration with Electron tutorial #1718
Conversation
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks! If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact [email protected] if you have any questions. |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
"homepage": "./", | ||
"scripts": { | ||
"react-start": "BROWSER=none react-scripts start", | ||
"electron-start": "ELECTRON_DEV=1 electron .", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably ask user to put these in .env
file because this syntax is not cross-platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #1712 (comment) though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The BROWSER
env variable can easily be moved to a .env
file but the ELECTRON_DEV
flag can be problematic because for the user it means changing the env variable value when building or having multiple .env
files and some loading logic to switch between prod and build.
What do you think of electron-is-dev
instead of the ELECTRON_DEV
flag?
I believe people will become confused when they try to use certain electron features, such as IPC. This really doesn't work unless we switch our webpack config to target |
@Timer Should I submit a PR with extra logic to change webpack's target (with a command line option, an env variable or other...) or update the current PR with a warning regarding limited support of electron without ejecting? |
I'm not sure if we want to toggle webpack into electron mode. A warning regarding its limitations is probably best for now. |
I was just following this and got it working, but only realised once I'd finished that I'd installed the additional packages as dependencies and not dev dependencies. Should have been obvious in hindsight but I think it would help to make it explicit. |
I'm going to punt this one until 0.10 because I'm not really happy with that fact that it doesn't "just work" for all cases. I don't want to recommend something that half works. We'll reconsider it in 0.10 or maybe think of a work around. |
"private": true, | ||
"devDependencies": { | ||
"concurrently": "^3.4.0", | ||
"electron": "^1.6.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nb electron-is-dev
should be in package.json somewhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it needs to be in dependencies
Thank you @poksme for this CRA-Electron write up!! I'm using it and it works well so far. I'm using the |
Works great for me |
Do people still use |
works great for me!! |
} | ||
``` | ||
|
||
Additionally, you can prevent `react-start` script from opening a browser window by setting the `BROWSER` env variable to `none`. On way of doing it is to create a `.env` file at the root of your React application folder with the following content: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo On
-> One
|
||
Once this file is created, the `package.json` needs to be edited: | ||
- Set the Electron script (here `main.js`) as `main`. | ||
- Set application's root (`./`) as `homepage`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering: What does this and why is it needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so that the CRA's production build will resolve index.html's resources path to be related to the current index.html rather than server'root
Since |
Went through the instructions ☝️ and had my react-scripts app (simple frontend template) wrapped in Electron within an hour. Will continue experimenting and contribute if I come across anything that could help! |
Oups, facing an issue with
Weird, I've tried adding
*(tried both in dependencies and devDependencies... cleaned-up node_modules and yarn install) Any ideas that can help ? |
Closing this for reasons mentioned above. I believe other tools are better suited for this. Thanks! |
static resources like images do not work.
Neither fetching data using relative paths is working. e.g.
It will try to access them using an absolute path starting with So instead of trying to access at: Same for the image: Any ideas on how to resolve that? |
As seen here https://twitter.com/poksme/timelines/838005727440470016