-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
WIP: use esbuild for building main renderer code #722
base: master
Are you sure you want to change the base?
Conversation
in ci, electron build attempts to rebuild [email protected], which is only used as a dev dep. should potentially be okay to omit
- use window.electron to access electron imports - use require for importing external modules (mostly node built-ins)
This reverts commit 163a0ab.
} | ||
} | ||
|
||
await runCommand(extractArgs()) |
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.
top level await
...the future is now
{ in: 'src/renderer/app.js', out: 'app' }, | ||
{ in: 'src/renderer/components/MapFilter/index.js', out: 'map-filter' }, | ||
{ in: 'src/renderer/components/MapEditor/index.js', out: 'map-editor' }, |
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.
app.js
loads home.js
, which loads map-filter
, and map-editor
. Does esbuild just know that, and keeps map-filter
and map-editor
out of the app
bundle? Or do we need to indicate that in the home.js
file like we do with webpack?
Goals
Notable changes (so far)
esbuild.mjs
. all configuration and any related build steps are performed there.build
: runs the bundler process in its entirety and output to thestatic
directory. accepts a--mode
arg of eitherdev
(default) orprod
. as of nowprod
will include minification, but should do other things when we're ready for that.watch
: runs the bundler process in watch mode, which will create a dev server that serves thestatic/
directory. accepts a--mode
arg of eitherdev
(default) orprod
. as of nowprod
will include minification, but should do other things when we're ready for that.clean
: cleans built assets that are outputted in thestatic/
directory./esbuild.mjs build
: compiles the renderer code and outputs it to thestatic/
directory./esbuild.mjs watch
: compiles the renderer code incrementally and starts the dev server that serves thestatic/
directory (defaults tohttp:/localhost:8000
).main.html
file. this allows for live-reloading (note, not hot module reloading or react fast refresh). see thepreload/main.js
preload script to see how that's done (it's just an eventsource that callslocation.reload
when a change is detected).src/renderer
directory. if you make changes to source code in the main process, background processes, or preload scripts, you'll still have to manually stop restart the server to see those changes.src/preload
directory. we can choose to bundle/process these but for now, we don't because they work in a different environment (access to both browser and Node apis because Node integration is turned on).static
directory. this way we don't deal with configuring loaders for certain assets right now, which was causing some asset duplication.react-intl
uses for auto id-ing messages based on file path. as a result, the flow for defining messages is identical to how we do it in Mapeo Mobile. i had to update every call todefineMessages
to manually specify something like{ id: ..., defaultMessage: ...}
for each message string. Based on the lack of a diff in the generatedmessages/
, i think my change has been completed successfully. Just need to keep this in mind moving forward.main.html
)window
in themain.js
preload script, which like a partial recommendation for when node integration is enabled (Error while importing electron in react | import { ipcRenderer } from 'electron' electron/electron#9920 (comment))Known issues
Let it be known that I'm not a bundling wizard. It's very possible that the build configuration I've implemented so far needs major reworking based on how browsers and Electron code work. Some other notable issues include:
external
have to be imported viarequire
when used in the main renderer code. for example, most of the node apis are listed as external and can only be used by doing something likeconst path = require('path')
. this is not really a blocking issue, but just a notable change that may be slightly inconvenient.Remaining work
path
module such as building a file path viapath.join
, it should be abstracted to a file located somewhere likesrc/renderer/node/path.js
as a helper function, which is then used by the component. my reasoning for this:builder.config.js
config to make sure that changes in output are accounted for in terms of what needs to be included/excluded when building the electron app. i'm sure i've missed some stuff there since i haven't specifically tested or focused on it yet.