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

[multi] Introduce preload script #3397

Merged
merged 7 commits into from
May 25, 2021
Merged

Conversation

matheusd
Copy link
Member

@matheusd matheusd commented Apr 21, 2021

Rebased on top of #3366

This simplifies some code that runs in the main process and introduces a preload script that runs before the main wallet interface. This starts the work towards moving all node-related calls to the preload script, so that node integration can be turned off for the main UI code.

Notable changes:

  • Switch from axios to standard fetch calls
    • axios still uses the old XMLHTTPRequest which is not needed in modern browsers. fetch allows more config options and removes the need for an additional dependency. (dropped this to send into a separate PR since it affects all third party calls)
  • Start to replace send/on IPC calls with invoke/handle for comms
    • invoke/handle automatically promisify calls and are asynchronous, thus removing the need for an error-prone architecture using send/on for async comms with the main process
    • Note that not all calls were converted yet: only enough for other devs to review and confirm the approach is sound. Other PRs will follow further converting calls and simplifying code.
  • Move fs related code to separate file
    • Instead of the UI code directly using the node fs module, the operations are shimmed through a local js module and call sites are updated accordlingy
    • This helps in moving the code to be referenced from preload-introduced functions, vs direct module require/import calls.
  • Introduce preload script to populate the fs functions

Preload scripts are specified by the main electron process code when creating a BrowserWindow object and run before the document specified by loadURL is loaded. The main usefulness for preload scripts is (re-)introducing select node and electron APIs to the UI code when the BrowserWindow is created with nodeIntegration: false and contextIsolation: true (which is the default in electron v12 and is likely to become mandatory in the future).

Unfortunately, due to the large number of places in the UI code where native node modules and electron API calls are happening today, it's going to be too difficult to write, test and review a single PR moving all code to the preload stage. Therefore, this is going to be broken up into a series of PRs that slowly improve the interface for IPC calls between the main and renderer processes and moves code to the preload script, such that a final PR can simply set the appropriate flags in the webContents during startup.

@matheusd matheusd force-pushed the improve-ipc branch 4 times, most recently from 5799411 to e31e5c1 Compare April 22, 2021 14:46
@matheusd matheusd marked this pull request as ready for review May 13, 2021 20:09
This makes the code cleaner in the ipc renderer side. ipcRenderer.invoke
implicitly returns a promise and doesn't block the ipcRenderer process
vs sendSync.
This is a pre-requisite to moving fs accesses to a preload script.
This introduces a preload script to the app. This script is generated via
a new webpack config, which watches over changes to regenerate it as needed
when running via `yarn dev`.

Currently the only API reintroduced via this preload script into the main
world is the wallet/fs module. In future commits, other APIs will be
compatibilized to the preload script and will be offered through it.
This serves as a check throughout development that the preload script is
behaving as expected.
This ensures tests pass.
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.

None yet

2 participants