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

Add HMR #25

Merged
merged 20 commits into from
Sep 26, 2022
Merged

Add HMR #25

merged 20 commits into from
Sep 26, 2022

Conversation

Jonghakseo
Copy link
Owner

No description provided.

Copy link
Contributor

@JunyWuuuu91 JunyWuuuu91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one question

package.json Outdated Show resolved Hide resolved
src/reload/client.ts Outdated Show resolved Hide resolved
@Jonghakseo Jonghakseo mentioned this pull request Aug 23, 2022
@bowencool
Copy link
Contributor

I suggest you to extract HMR runtime codes into virtual module, you can refer to these codes:
https://vscode.dev/github/Jervis2049/vite-plugin-crx-hot-reload
https://vscode.dev/github/yeqisong/vite-plugin-vue-crx3

@Jonghakseo
Copy link
Owner Author

I suggest you to extract HMR runtime codes into virtual module, you can refer to these codes: https://vscode.dev/github/Jervis2049/vite-plugin-crx-hot-reload https://vscode.dev/github/yeqisong/vite-plugin-vue-crx3

It looks good to apply. Let me take a closer look and try to apply it. thank you

@JunyWuuuu91
Copy link
Contributor

I have a question whether it can be done that refresh background.js when i modified it ?
So far, when I change the code, it doesn't work even if I manually refresh the crx to load it. Then you must uninstall the crx and reinstall it。have any idea? maybe my wrong @Jonghakseo

Copy link
Contributor

@JunyWuuuu91 JunyWuuuu91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GLTM

@Jonghakseo
Copy link
Owner Author

Hello everybody. Sorry for the delay in merging this PR.

I need to review and supplement the suggestions you have made.
However, due to personal circumstances, I have not been able to work for the past two weeks.
I'm trying to spend enough time working through the holidays next week. (Thanksgiving Days)

Thank you for your understanding.

@JunyWuuuu91
Copy link
Contributor

Happy holidays!

Copy link
Contributor

@JunyWuuuu91 JunyWuuuu91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the slow review, as I have been busy with the company's work content recently。

GLTM !

have two question:
1、chrome always give a error about look like can't load file: src/pages/options/index.html and close crx when hrm reload。Then must goto chrome://extensions/ page click refresh button by my self。Maybe chrome checked local file between vite transformed and render files execution。(only happened when i write refreshOnUpdate function in background.js
2、why use rollup plugin alone but not vite ?

@Jonghakseo
Copy link
Owner Author

Jonghakseo commented Sep 16, 2022

@JunyWuuuu91

Hi! I'll give you an answer.

1️⃣

Yes, that's right. From what I found out, the only way to update the background.js was to call Chrome.runtime.reload. The problem is that invoking Chrome.runtime.reload will have the same effect as reinstalling the entire extension, not just the service worker.

To resolve this issue, the reloadServer must be able to know if the change occurred in the background or in the client.
However, in order to use this method, you need to track which file/path the change event occurred inside the src folder.

If you look at the screenshot below, you can actually see other sources using those solutions.

스크린샷 2022-09-16 오후 8 24 21

utils/server.js

The cons of this method is that it takes away the opportunity for the user to change the file name freely. Well... anyway, vite config is still detecting the location of the files inside the src folder :)

Or, I think we can change the name of the file used in each part in one place while changing the boiler plate more rigidly.

Anyway, I'm thinking about it, so I'd appreciate it if you could give me your opinion.

2️⃣

I didn't quite understand this question. I'm not sure exactly what it means to use the rollup plug-in and not use the vite plug-in.

@JunyWuuuu91
Copy link
Contributor

JunyWuuuu91 commented Sep 19, 2022

@Jonghakseo
1️⃣
I think the solution that screenshot above is OK, All of crx folder has a clear meaning like as backgroundcontent ...。
Of cource,plugin also can provider a attribute to modify the folders name before detect.

2️⃣
I meaning why do not use vite to implement the reload plugin,Is it impossible?Because i seen that u create a config file with the rollup

@Jonghakseo
Copy link
Owner Author

@Jonghakseo 1️⃣ I think the solution that screenshot above is OK, All of crx folder has a clear meaning like as backgroundcontent ...。 Of cource,plugin also can provider a attribute to modify the folders name before detect.

2️⃣ I meaning why do not use vite to implement the reload plugin,Is it impossible?Because i seen that u create a config file with the rollup

1️⃣

All right, but I want to give developers a more flexible reload function without checking or modifying their internal implementation. Therefore, I worked on setting the watch path so that I could set the path to trigger the update.

2️⃣

I'm not sure about the additional benefits of using the vite plug-in. There is no reason not to use the vite plug-in if it is more improved when using other hooks. If you have a good idea, I would appreciate it if you could suggest it.

For now, HMR will merge.

@Jonghakseo Jonghakseo merged commit 1619b47 into main Sep 26, 2022
@Jonghakseo Jonghakseo deleted the add-hmr branch September 26, 2022 03:01
@nomi-san
Copy link

Seems your HMR doesn't work correctly 😅

Look around this web ext plugin:
https://github.com/samrum/vite-plugin-web-extension

They just emit HTML pages into ext file during dev server starts.
This works as normal Vite React app.

Let's create a custom HTML entry and add it to rollup's input.

src/
|__ index.html
|__ main.ts
  <script type="module">
  <!-- HMR here -->
import RefreshRuntime from "/@react-refresh"
RefreshRuntime.injectIntoGlobalHook(window)
window.$RefreshReg$ = () => {}
window.$RefreshSig$ = () => (type) => type
window.__vite_plugin_react_preamble_installed__ = true
</script>

  <script type="module" src="/@vite/client"></script>
   ...
 <script type="module" src="./main.ts">

Any TS file is imported by the entry will be transpiled, even accessing it through HTTP.

// main.ts ; https://localhost.../main.ts
// transpiled JS code here

@Jonghakseo
Copy link
Owner Author

Seems your HMR doesn't work correctly 😅

Look around this web ext plugin: https://github.com/samrum/vite-plugin-web-extension

They just emit HTML pages into ext file during dev server starts. This works as normal Vite React app.

Let's create a custom HTML entry and add it to rollup's input.

Wow. Here's the implementation I was looking for.
It seems very efficient.

I will refer to it and reflect it.
thank you.

@Jonghakseo Jonghakseo self-assigned this Jan 4, 2023
@@ -15,10 +15,13 @@
"allowSyntheticDefaultImports": true,
"lib": ["dom", "dom.iterable", "esnext"],
"forceConsistentCasingInFileNames": true,
"typeRoots": ["./src/global.d.ts"],
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curios what is it about

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

5 participants