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

Replace webpack with Es Build #726

Open
4 tasks
ErikSin opened this issue Jun 7, 2023 · 1 comment
Open
4 tasks

Replace webpack with Es Build #726

ErikSin opened this issue Jun 7, 2023 · 1 comment

Comments

@ErikSin
Copy link
Contributor

ErikSin commented Jun 7, 2023

For a better developer experience, upgrade webpack to esbuild.

Goals

  • Improve dev experience for writing renderer code. Initially thought that this would also be a good opportunity to investigate bundling the main process code, but decided against it because 1) we don't currently do that so it would be increasing the complexity of this work and 2) previous experiments suggest that Rollup may be a better fit for bundling node code.
    • things within scope include but are not limited to: supporting TypeScript, auto reload on source changes (renderer only), faster build times.
  • Reduce tooling complexity and dependency. Things like Babel, Webpack, etc tend to add a lot of maintenance overhead and subject us to keeping up with ecosystems to fix things or maintain compatibility. This work is an attempt to understand if having more control in these processes could be more maintainable for us moving forward, without relying as much on the ecosystem.

To do

  • Continue work of WIP: use esbuild for building main renderer code #722
  • (Taken from Andrew in WIP: use esbuild for building main renderer code #722 ) Consolidate any renderer code that uses node apis in a specific directory in the renderer. for example, if a component does some stuff with the path module such as building a file path via path.join, it should be abstracted to a file located somewhere like src/renderer/node/path.js as a helper function, which is then used by the component. my reasoning for this:
    • it makes it easier to understand which components rely on node apis. right now, it's a matter of knowing what deps we have that are node-specific and then grepping the codebase to see which renderer components import the various deps. with this re-organization, it will be much easier to just check for files that import from this node-specific directory. this approach probably won't work for all node-specific deps that we use, but if we can cover most of them, i think that's significantly helpful.
    • having the separation will make it easier to eventually move away from the enabled node integration for the renderer process. it's highly recommended against and there are pretty good references for how to go about it (see Error while importing electron in react | import { ipcRenderer } from 'electron' electron/electron#9920 (comment)), but they require changes in the renderer and main process code.
  • (Taken from Andrew in WIP: use esbuild for building main renderer code #722 ) Remove all of the webpack remnants. i haven't done this yet because i wanted a reference to the previous config to make sure the new process covers what we previously had.
  • (Taken from Andrew in WIP: use esbuild for building main renderer code #722 )Update the 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.
@ErikSin
Copy link
Contributor Author

ErikSin commented Jun 13, 2023

I spoke to Andrew about this and some of the bundler complication comes from the way we use node-integrations, Message Ports, and Renderer Processses for mapeo-core. With the work being done on #725, this will hopefully be simplified with Mapeo-Core-Next. Therefore, it would make sense to do this work after mapeo-core-next is integrated, and this will hopefully simplify the bundling.

I also tried to do some of this work this weekend, and realized it is a little out of my wheelhouse. I think it would make sense for me to do this work with someone as I cannot do it myself and I would like to learn it.

With all that considered, I am going to take this off sprint 8 and work on it, with someone else, on a later sprint.

@ErikSin ErikSin removed the sprint 8 label Jun 13, 2023
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

No branches or pull requests

1 participant