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

Enhancement for reload #1092

Open
breadgrocery opened this issue Oct 19, 2024 · 2 comments
Open

Enhancement for reload #1092

breadgrocery opened this issue Oct 19, 2024 · 2 comments
Labels

Comments

@breadgrocery
Copy link
Contributor

breadgrocery commented Oct 19, 2024

Feature Request

Currently, the reload function does too little, it only reloads the Inline-Config, nothing more. But the configuration is not all defined in the Inline-Config, they may be configured in different modules.

It is not ideal to re-run the whole life cycle when a reload occurs, but the modules are not even aware that a reload has occurred, which leads to the modules not being able to respond to the reload as they should.

Edit: Somewhat similar to #939

  wxt = {
    config,
    hooks,
    hook: hooks.hook.bind(hooks),
    get logger() {
      return config.logger;
    },
    async reloadConfig() {
       wxt.config = await resolveConfig(inlineConfig, command);
    },
    pm,
    builder,
    server
  };

For example, the i18n module requires the default-locale configuration. However, when reloading, if default-locale is not configured through Inline-Config, there will be no default-locale configuration in the reloaded configuration, and it will fail in the prepare:types phase.

Summary:

  • Modules are not aware of reloading and cannot respond to reloading

What are the alternatives?

My idea is to add a series of hooks about reload (reload:before and others) so that the module can know that a reload is happening and respond correctly when the reload occurs to avoid some problems brought by HMR.
In this way, no need to rerun the entire life cycle, only the pre-defined functions need to be executed.

    await reloadConfig();
    wxt.hooks.CallHook("reload:before",()=>{});
    ......
     wxt.hooks.CallHook("reload:done",()=>{});

The following is my temporary solution, reconfigured in a relatively early hook.However, this is not a proper or legitimate solution.
The reason I configure the manifest in a separate module is that when a manifest entry is configured that is not supported by the browser, warnings will be given, and I don't want those warnings.

import process from "node:process";
import { defineWxtModule } from "wxt/modules";

const permissions = {
  common: ["downloads", "downloads.open", "storage"],
  chrome: ["background", "downloads.shelf", "downloads.ui", "offscreen"],
  firefox: []
};

export default defineWxtModule(wxt => {
  const { browser } = wxt.config;
  const configure = () => {
    wxt.config.manifest = {
      "name": "__MSG_extension_name__",
      "description": "__MSG_extension_description__",
      "default_locale": "en",
      "version": process.env.npm_package_version,
      "permissions": [
        ...permissions.common,
        ...permissions[browser as keyof typeof permissions]
      ].filter(Boolean),
      ...(browser === "chrome" && {
        "minimum_chrome_version": "88"
      }),
      ...(browser === "firefox" && {
        "browser_specific_settings": {
          "gecko": {
            "id": "download-manager@breadgrocery.github.com",
            "strict_min_version": "109.0"
          }
        }
      }),
      ...wxt.config.manifest
    };
  };
  configure();
  wxt.hooks.hook("prepare:types", configure);
});
@aklinker1
Copy link
Collaborator

I am planning on completely restarting the entire context when config changes so module setup functions re-run automatically.

It is not ideal to re-run the whole life cycle when a reload occurs

Seems like the ideal solution to me, did you see something in the code that would make you think this?

@breadgrocery
Copy link
Contributor Author

Seems like the ideal solution to me, did you see something in the code that would make you think this?

No. I think this because I see that you only do simple things in the reload step, and I think you may be considering performance or other issues. If there is no problem, then that would be best.

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

No branches or pull requests

2 participants