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

Revive Polykey Desktop #149

Open
wants to merge 8 commits into
base: staging
Choose a base branch
from
Open

Revive Polykey Desktop #149

wants to merge 8 commits into from

Conversation

CMCDragonkai
Copy link
Member

@CMCDragonkai CMCDragonkai commented Jul 24, 2022

Description

This PR attempts to revive Polykey Desktop. It's been about a year and we are interested in making some changes and having a base to develop UI interactions like for debugging.

Some things I'm experimenting with here:

  • Using esbuild instead of webpack, webpack is quite complex
    • There's a builder for main and renderer
    • Main code is nodejs code
    • Renderer is SPA code
    • Main renderer has to target nodejs
    • Nodejs is bundled by electron, so it has to target the that bundled nodejs version (this seems changeable if we were to compile electron from scratch)
    • Renderer building is more complexing involving "loaders" and vue, react...., css and more
  • Due to the primacy of react native, a conversion of vue components into react components, and also reduction of the amount of dependencies
  • Esbuild doesn't use tsc, it does however automatically read the tsconfig.build.json, which it is configured to do so
  • This means any build process that is intended to go into production should be doing tsc -p ./tsconfig.json which doesn't do any building. This also means a bunch of tsconfig isn't really needed like incremental compilation and such. The esbuild just strips types and does its own build. The tsc is left just for type checking.
  • The renderer will have an index.html this has to be produced from esbuild. To do this, we need to use https://www.npmjs.com/package/@craftamap/esbuild-plugin-html in a similar way to how the original html webpack plugin used to work.
  • This will all be done through a scripts/build.js, this reduces the number of dependencies we need by a significant amount.
  • We are expecting to to produce an electron package with no dependencies. All of the main code will end up in a single file, and all of the JS code will end up in one file too. There will be no node_modules, so we need to adjust the default.nix
  • Lots of things learned from Nix.
  • It is quite likely this code will depend on polykey as that is the core library.
  • Experiment with the preload.js to provide a context bridge between main and renderer, so the renderer can call the RPC functionality directly from the main process instead of calling it using browser APIs.

Issues Fixed

  • Fixes #...

Tasks

  • 1. ...
  • 2. ...
  • 3. ...

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

@ghost
Copy link

ghost commented Jul 24, 2022

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@CMCDragonkai CMCDragonkai self-assigned this Jul 25, 2022
@CMCDragonkai
Copy link
Member Author

So esbuild is introduced as one extra tool here replacing webpack and all the plugins. This is due to "layer collapsing" in the JS ecosystem.

However swc is an alternative transpiler... and we now have 3 transpilers all running... swc for ts-node, esbuild own transpiler and tsc which is used by jest. Further layer collapsing is possible by making jest use @swc/jest rather than tsc. This will depend on swc-project/swc#1348 (comment), and then jest could benefit from the same speed improvements of swc.

Note that esbuild would be a viable intermediate tool before running pkg over it to give us better control over the bundling step. Note vercel/pkg#181. I do wonder how https://github.com/vercel/ncc fits in the picture. It appears to also act like a bundler. Then pkg can take that JS file and "bundle" it again with the nodejs executable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant