-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add pre-request tab and minimal execution context #7065
Conversation
50eaa1f
to
d25706d
Compare
412bcd2
to
aa4121b
Compare
sandbox: true, | ||
contextIsolation: true, | ||
nodeIntegration: false, | ||
contextIsolation: false, |
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.
This is awesome to simplify parts such as message channel creation and message handling in preload. And I would propose to enable those flags if possible, as this environment is mainly for those anonymous code and you know it would be difficult for us to turn on them again :). I also spent lots of time to keep them enabled in another PR, please consider this.
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.
This is an idea for a developer experience tradeoff. I have also spent many hours writing bridges in the app to prepare for contextIsolation. Its something I want to investigate but it requires more boilerplate and I don't want to add anymore until I can deliver a more seamless dx. The faster it is to work on the more secure we can make it.
So far I enabled hot reload of changes to the hidden window index.ts on change without restarting the app. The next step is to stop vite from reloading the main browser window when we change the hidden window.
After this is will be easier to experiment with the security flags
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.
Ok thanks for the push @ihexxa ive reenabled contextIsolation, and pointed the index.html directly at the index.ts. The dx is improved and only preload changes require a full reload. However i still havent stopped insomnia from reloading on hidden window changes but i found a workaround. If you logout it doesnt go back to workspace view at reload so the learning loop is faster.
|
||
invariant(mainWindow, 'mainWindow is not defined'); | ||
mainWindow.webContents.mainFrame.ipc.on('open-channel-to-hidden-browser-window', event => { | ||
const { port1, port2 } = new MessageChannelMain(); |
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 may close
these ports to avoid leaking and take care of crash of the window, as they are related to stability and user experience.
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.
I want to scope these meta aspects out into testable PRs where possible ill add it to future work
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.
👍 thanks for understanding, it also makes sense to me and let's test it later as probably it is not necessary.
2e43cff
to
141adec
Compare
* works * add preload * promise api * tidy * create hash works * return errors from worker * basic wiring * remove preload * move build output to folder * async execution * add simple context object * smoke test * fix types * fix unit tests * remove createHash remote function * tidy * naming * fix test * add errors to preview and timeline * basic require support * fix types * fix warning * can get logs * fix console.log patch * logs * fix test * remove log * skip mock test * fix test * fix types * can write to timeline * add watcher * improved dx * can console.log in the script * simplify * enable contextIsolation * fix types * use vite for hidden window build and dev * fix type * rename * move pre request test to critical * fix typo * close ports * unskip test
inspired by: https://www.electronjs.org/docs/latest/tutorial/message-ports/#worker-process
And #6991
provides a promise based API for the insomnia renderer to send a script and request related data to a hidden browser window and receive the request data back mutated by the script.
Goals:
todo
future work
closes INS-3380