Allow mutilple sources for "web-ext run --source-dir=..."

[piroor]

Is this a feature request or a bug?

This is a feature request: I need an ability to load multiple temporary addons at a time.

What is the current behavior?

Now I'm developing two addons treestyletab and multipletab. The treestyletab provides some APIs for other addons, and multipletab provides some features based on these APIs. Because both addons are still in development, APIs are not stable and I have to edit both addons when I find some problems around APIs.

Web-ext now allows me to start Firefox with just a specific in-development addon as a temporary addon, however I need to load two (or more) addons in this case. Such a usage seems impossible for now.

What is the expected or desired behavior?

web-ext should accept multiple source dirs, like:

web-ext run --source-dir=PathToAddonA,PathToAddonB,...

or

web-ext run --source-dir=PathToAddonA --source-dir=PathToAddonB ...

[kumar303]

Hi. I think this will be possible after #830 although that patch was meant more for web extension experiments. It will definitely allow you to install multiple extensions at once though.

@rpl is working on that patch but I think he has Android support in the queue for completion first.

[piroor]

It sounds good! I close this and I'm waiting for the feature. Thanks!

[kumar303]

Actually, I think it would be good to keep this issue open just to make sure this use case is covered. The use case in #830 is for experiments which is a bit of a different case.

[piroor]

#830 was closed without merging, so the --experiments is not available on web-ext run and I still need something way to do this.

[ygmarchi]

I need also need this feature. How do you get by in the meantime?

Use case: Selenium IDE is a Firefox extension to register user activity on a page and replay it. It allows plugins which are Firefox extensions themselves. Selenium IDE is a work in progress and while trying developing its own plugin one may want trying implementing new features in Selenium IDE itself. So one may wish to be able to load and debug both extensions at the same time.

[piroor]

Just as a workaround I'm using a forked version of web-ext. I think it may helps you: https://github.com/piroor/web-ext/wiki/Installation

[ygmarchi]

Hello, I'm trying it right away. Thank you very much, Carlo. Il ven 17 gen 2020, 14:22 piroor <notifications@github.com> ha scritto:

…

Just as a workaround I'm using a forked version of web-ext. I think it may helps you: https://github.com/piroor/web-ext/wiki/Installation — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1107?email_source=notifications&email_token=AAMSYF45D2PTRES3CKMXPLTQ6GWKPA5CNFSM4D7FK54KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJHVEEY#issuecomment-575623699>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMSYF2PBMEOXPNYCSATYGDQ6GWKPANCNFSM4D7FK54A> .

[2br-2b]

Are there any plans to bring this feature upstream?

[ankushduacodes]

@rpl I would love to take on this issue on your suggestion over at matrix channel.

Do you any pointers on where to get started with this?

[rpl]

@rpl I would love to take on this issue on your suggestion over at matrix channel.

Do you any pointers on where to get started with this?

@ankushduacodes Sure thing! Follows some initial pointers (there may be more to say, but let's leave additional details to after you started to think about it based on the initial details and you may get some more specific questions and/or a draft pull request):

• First of all, the underlying components that do actually run the extension and reload it are defined in src/extension-runners

  • the internals defined there do actually support already an array of extension source dirs, but we don't expose it at the moment on the cli, e.g. see

    web-ext/src/extension-runners/base.js

    Line 15 in 706afa3

    extensions: Array<Extension>,

  • giving a look to these internals will be definitely helpful to allow you to see a more complete picture of how that part works and what kind of changes we may need to apply in the web-ext sources (and there is a small chance that there may be also some small tweaks on these internals needed, but we will definitely notice if that is the case while planning the changes needed)

• source-dir cli option is currently defined as a string, and so it wouldn't be allowed to be passed more than once, if we want to allow web-ext run to be called for more than one source dir then we may need to change that to array... but:

  • source-dir is a global cli option and so the same option is also being used by the other commands, we may want to evaluate if we want (and if it does really make sense) to support multiple extensions also for the other commands (in particular build, lint and sign)
  • command implementations and their type annotations will have to be updated to expect sourcedir to be an array of strings

• web-ext doesn't also support loading default for the cli options from a config file, and we should double-check if there are other changes needed in the config loading logic to avoid breaking config files that do include the sourceDir config option as a string (e.g. we may need to detect that and convert the option value from string to an array of one string)

Let me know if you have more question and/or doubts once you got a chance to look into this initial pointers and think about them a bit.

[ankushduacodes]

Hi @rpl,

First and foremost, My apologies for sitting on this issue for too long. In the middle of online exams and now online classes and classwork, I just couldn't squeeze some time out. I hope you will understand :).

That being said, I have finally gone through the initial pointers and recommended code, and it makes sense so far. And I have made a draft #2225 to track progress on this issue

I have also noticed that when we create the watcher here we are making reloadExtension function load the extension by sourceDir, which at this moment is string which means only when that content of that particular extension changes, we reload. but if sourceDir is an Array<string>, then what would be the reloading strategy.

Confusion(s):

1. I understand that we can load one extension on multiple targets at the same time, I want to confirm if a single extension runner can manage multiple extensions on ONE particular target, for example: let's say (Hypothetically for now) we are to load two extensions on firefox-desktop which creates an extension-runner, can that extension runner manage all two of the extensions? (It seems to be the case from what I have read in the code so far from extension-runners/index.js#L116 and extension-runners/firefox-desktop.js#L99 but I am confirming it for good measure)

web-ext doesn't also support loading default for the cli options from a config file, and we should double-check if there are other changes needed in the config loading logic to avoid breaking config files that do include the sourceDir config option as a string (e.g. we may need to detect that and convert the option value from string to an array of one string)

2. Could you please elaborate on above comment, I am not able to understand what exactly do you mean by the config files and the above comment in general.

Question:
I think I have a greater picture of what needs to be done here, but I am not sure where to start making changes as any change to sourceDir will break whole bunch of functionality and test?, any suggestions will be much appreciated.

Please let me know If I may have missed some important details related to this issue.
Thank you :)

[rpl]

Hi @rpl,

First and foremost, My apologies for sitting on this issue for too long. In the middle of online exams and now online classes and classwork, I just couldn't squeeze some time out. I hope you will understand :).

@ankushduacodes Sure thing, I'm always super happy to see you working on bugs and enhancement to the projects that I work on, and you are a very reliable contributor, but you should also feel absolutely free to pause your contribution whenever you need to and then come back when possible again.

That being said, I have finally gone through the initial pointers and recommended code, and it makes sense so far. And I have made a draft #2225 to track progress on this issue

I have also noticed that when we create the watcher here we are making reloadExtension function load the extension by sourceDir, which at this moment is string which means only when that content of that particular extension changes, we reload. but if sourceDir is an Array<string>, then what would be the reloading strategy.

Confusion(s):

1. I understand that we can load one extension on multiple targets at the same time, I want to confirm if a single extension runner can manage multiple extensions on ONE particular target, for example: let's say (Hypothetically for now) we are to load two extensions on `firefox-desktop` which creates an extension-runner, can that extension runner manage all two of the extensions? (It seems to be the case from what I have read in the code so far from [extension-runners/index.js#L116](https://github.com/mozilla/web-ext/blob/master/src/extension-runners/index.js#L116) and [extension-runners/firefox-desktop.js#L99](https://github.com/mozilla/web-ext/blob/d07b9238ed91c9bc8f475d9890b0509e12fe4350/src/extension-runners/firefox-desktop.js#L99) but I am confirming it for good measure)

We do have one extension runner per target, and a MultiExtensionTarget that wraps multiple per-target extension runner into a class instance that provide the same interface of the per-target extension runner.

If you look to the flow type defined here:

web-ext/src/extension-runners/base.js

Lines 8 to 40 in 8bd0d13

	export type Extension = {|
	sourceDir: string,
	manifestData: ExtensionManifest,
	|};
	export type ExtensionRunnerParams = {|
	// Common cli params.
	extensions: Array<Extension>,
	profilePath?: string,
	keepProfileChanges: boolean,
	startUrl: ?string | ?Array<string>,
	args?: Array<string>,
	// Common injected dependencies.
	desktopNotifications: typeof defaultDesktopNotifications,
	|};
	export type ExtensionRunnerReloadResult = {|
	runnerName: string,
	reloadError?: Error,
	sourceDir?: string,
	|};
	export interface IExtensionRunner {
	getName(): string,
	run(): Promise<void>,
	reloadAllExtensions(): Promise<Array<ExtensionRunnerReloadResult>>,
	reloadExtensionBySourceDir(
	extensionSourceDir: string
	): Promise<Array<ExtensionRunnerReloadResult>>,
	registerCleanup(fn: Function): void,
	exit(): Promise<void>,
	}

you'll notice that every per-target extension runner:

• will get an array of Extensions objects (which is basically just a js object with two properties: the manifest data and the source directory path)
• does have both a reloadExtensionBySourceDir and reloadAllExtensions

My rough idea would be that:

• we could have one watcher per extension dir
• if an extension is changed we have to reload only that one on all the per-target extension runner

Ideally we may be able to just call on the MultiExtensionTarget instance the reloadExtensionBySourceDir method, passing the sourceDir of the single extension that the watcher instance was related to, but I don't exclude that it may need "some (hopefully small) refactoring "here and there" ;-)

web-ext doesn't also support loading default for the cli options from a config file, and we should double-check if there are other changes needed in the config loading logic to avoid breaking config files that do include the sourceDir config option as a string (e.g. we may need to detect that and convert the option value from string to an array of one string)

1. Could you please elaborate on above comment, I am not able to understand what exactly do you mean by the config files and the above comment in general.

eh, first of all, in my comment there was a typo: web-ext doesn't ... => web-ext does ....

I admit that my comment could have been clearer :-P, I could have phrased that in a way simpler way, let me try:

• web-ext can load cli options from a config file
• and so if sourceDir becomes of type array it may make this a backward incompatible changes from a config file perspective

in other words: "if a developer has a config file where sourceDir is a string, then our changes to the option type may likely make that file to not be loaded successfully anymore, and we may have to special case it by converting the string value into an array of strings when we load the file and log a warning"

Question:
I think I have a greater picture of what needs to be done here, but I am not sure where to start making changes as any change to sourceDir will break whole bunch of functionality and test?, any suggestions will be much appreciated.

Please let me know If I may have missed some important details related to this issue.
Thank you :)

I know that feeling, when every developer tools in the project (eslint, flow, unit tests etc) are all yelling at you at the same time and there is no direction to take that would "please" all of them at the same time.

I can assure you that, while you grow your experience, you will manage to figure out how to focus your attention on a subset of those tools and errors when you need to (which one to focus on chosen with actual reasons, I'll try to explain that below in a bit more detail), and then go back to ensure that "they are all happy again" as one of the final steps.

let me try to describe my approach in words:

• while planning the changes needed, it is normal that there will be test failing and new lines not covered
• during the planning/design phase I don't care too much that the build isn't green, but I do use what is failing as an hint of things that:
  • I may need to change/fix to call the set of changes complete and ready for a review (and I don't care about those in the initial planning/design phase)
  • I may need to take into account to fix the design / plans

in other words, "not all the failures you get during this phase are equally important", some could actually be useful to inform you about details that may not work out as you were planning (you want to possibly spot these ones and use those hints to tweak the design), and others may just let you know that there will be more work to make the build green again (and don't need to be handled until you got the design right and you are ready to finalize the patch).

To go a bit more into the concrete kind of failures you may get:

• eslint:
  • eslint errors will be often just stylistic errors, I just run eslint --fix to fix those and see if there is any eslint error that does actually matter (e.g. something that is actually wrong and I would likely catch once I have started to add new tests to get more coverage)
  • flow type checks errors
    • flow errors may be a bit annoying while drafting some experimental changes and you don't want to annotate quick throw-away experiments with types (if that is the case I just skip flow, there should be an environment variables that we use in the windows CI jobs, you can use that locally to skip flow checks during these experiments)
    • flow is often able to tell you that something is wrong even before you write a test case and I personally like that (not all developers like it) and so when I think I've a good approach that I'm going to incrementally improve I take care of looking more carefully to the flow errors, annotate some more stuff to get better one and fix the ones that I can without spending too much time on them (I may fix all of them at this point, or leave some for later, when I'm doing the final tweaks)
  • unit tests
    • in some cases I draft a design quickly, don't care about any test or eslint and flow errors, I just build and run the tool interactively to get a sense if I think the direction is right, then I just dump those changes into a file and revert the repo to its clean state
    • at this point I start re-apply those draft changes properly:
      • I start from the lower level, and I take care only of making the existing unit tests that cover that specific part to pass (updating them if some of the expectations should change because of the change I'm doing), for this issue the lowed levels are the extension runners
      • then I move to the level right above it and I do the same (for this issue the level above it is the cmd/run.js module and then the yargs options and the config fine)
      • repeat until I'm done
    • now all the unit tests are usually passing, I can move into finalizing the patch, e.g.:
      • some parts may not be looking great yet, I ask myself questions like "did I took any shortcut during the design phase?", "could this use a small refactoring to better fit into the project?", "would I understand this if I was reading this code for the first time or reviewing this patch?" etc., and based on the answers I give to myself ("answering my own questions" sounds a bit crazy I know :-P, but it is actually useful ;-)) I may tweak the changes a bit more before taking care of the code coverage
      • the code coverage may still use some more love, especially in terms of the scenarios tested (not just from a "lines hit" perspective)
      • is there something that I would like to test too tricky to test in unit test? if yes and I do care about covering that as a test, I may add an integration test, if in doubt I defer that and I just make sure that the patch is clean and clear enough to open it for review

Obviously consider all above as my personal preference (actually, it is more like "me trying to describe my personal preferences for dealing with that phase where everything breaks even before I got enough changes to call it a patch" ;-)) and it is not an exact sequence of what every developers should be doing, you'll develop your own personal preferences, just take inspiration from what I described above to bootstrap your own, then you will have to keep tweaking your personal preferences based on your experience.

I hope the above list may contain some useful pointers to unblock you on this, but also feel free to reach me on Matrix (as I think that a chat may be a better fit for discussing further about this) and I'll be happy to discuss more about workflows tips and answer any other questions you may have.