-
-
Notifications
You must be signed in to change notification settings - Fork 275
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
fix: recreating keyboardShortcuts interface on file changes. #1465
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for creative-fairy-df92c4 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Was noticing this too but wouldn't this potentially be a memory leak since we keep creating it?
Yeah, now that I think about it, it really can be. as such there are already existing issues for windows. |
@wxt-dev/analytics
@wxt-dev/auto-icons
@wxt-dev/browser
@wxt-dev/i18n
@wxt-dev/module-react
@wxt-dev/module-solid
@wxt-dev/module-svelte
@wxt-dev/module-vue
@wxt-dev/storage
@wxt-dev/unocss
wxt
commit: |
const reloadOnChange = createFileReloader(server); | ||
const reloadOnChange = () => { | ||
keyboardShortcuts.start(); | ||
keyboardShortcuts.printHelp({ | ||
canReopenBrowser: | ||
!wxt.config.runnerConfig.config.disabled && !!runner.canOpen?.(), | ||
}); | ||
return createFileReloader(server); | ||
}; | ||
server.watcher.on('all', reloadOnChange); | ||
keyboardShortcuts.start(); | ||
keyboardShortcuts.printHelp({ | ||
canReopenBrowser: | ||
!wxt.config.runnerConfig.config.disabled && !!runner.canOpen?.(), | ||
}); |
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.
One side-effect to this change is that the help message is printed after every file is saved, right? Should we leave the printHelp
call outside the reloadOnChange
where it was before so we only print the message when the server is started/restarted?
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.
Do we know why the readline interface stops working when a file is saved? Or are we just making a change in hopes that it fixes the problem?
const reloadOnChange = () => { | ||
keyboardShortcuts.start(); | ||
keyboardShortcuts.printHelp({ | ||
canReopenBrowser: | ||
!wxt.config.runnerConfig.config.disabled && !!runner.canOpen?.(), | ||
}); | ||
return createFileReloader(server); | ||
}; | ||
server.watcher.on('all', reloadOnChange); |
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 think this change is still wrong... The file watcher args aren't being passed into the function returned by createFileReloader
properly.
This is the right way to do it:
const reloadOnChange = () => { | |
keyboardShortcuts.start(); | |
keyboardShortcuts.printHelp({ | |
canReopenBrowser: | |
!wxt.config.runnerConfig.config.disabled && !!runner.canOpen?.(), | |
}); | |
return createFileReloader(server); | |
}; | |
server.watcher.on('all', reloadOnChange); | |
const reloadOnChange = createFileReloader(server); | |
server.watcher.on('all', async (...args) => { | |
await reloadOnChange(...args); | |
// Restart keyboard shortcuts after file is changed - for some reason they stop working. | |
keyboardShortcuts.start(); | |
}); | |
keyboardShortcuts.printHelp({ | |
canReopenBrowser: | |
!wxt.config.runnerConfig.config.disabled && !!runner.canOpen?.(), | |
}); |
I also included moving the printHelp
call out so it's only printed once. See #1465 (comment)
Overview
I noticed that keyboard shortcuts weren't working after files were being saved or HMR was triggered.
Hope this fixes it.