express is transitively pulled by dependents of web-ext

[julienw]

Hey,

While updating to a new version of web-ext I was surprised that this pulled all of express. So I looked closer and noticed that this comes from this tool. This seems a bit too much that express is transitively pulled to all addons that would depend on web-ext, as it's a pretty big dependency.

Do you think this would be possible to remove this dependency? Maybe extract the functionality that needs express to a different package?

Thanks

[julienw]

From my limited understanding I don't see the express functionality used in addon-linters (used by web-ext), so my guess is that you use it in a serverside tool. Therefore it would be very good to split this functionality.

[willdurand]

Interesting side effect that we didn't anticipate, sorry about that!

I am not so sure about what to do, though. We started to use this package in addons-linter to avoid duplicate code and consolidate a few things. We don't really want to maintain too many repos but this is clearly a problem so we might have no other choice... @rpl WDYT?

[julienw]

I'm not sure of your use case exactly, but maybe this is a good use case for peerDependencies here? Especially if the project where you use this also depends on express explicitely!

[rpl]

Ouch, yeah... this seems a quite unfortunate side-effect (thanks @julienw for reporting it, I usually give a look to the renovatebot PRs in the web-ext repo to double-check nothing unexpected is being introduced in the package-lock.json file, but I honestly missed to notice this one).

From a quick look to the sources in this repo it seems that express is being used only by the src/functions.ts module, and so it looks that we may have a couple of (slightly different) options:

• moving express to a peer dependency and have the package that uses that module to import express as its own dependency (and maybe in functions.ts also check if the module is available and throw an error if it is not) as Julien also suggested.
• or moving src/functions.ts into a separate npm package by splitting it into a separate repo and npm package
• or turning this repo into a monorepo that does publish two separate npm packages (one for just the io utils, and one with the functions)

@willdurand how do the strategies briefly described above sound to you? is there any additional approach that you did think of during the past few days that may be also worth to be evaluated?

[julienw]

I do have a quick question: is the code in functions.ts reused more than once in some internal code? Obviously it's not used in addons-linter which is the only publicly dependent package, that's why I'm asking. If it's used only once, maybe this code could be moved to the internal project instead.

[willdurand]

@willdurand how do the strategies briefly described above sound to you? is there any additional approach that you did think of during the past few days that may be also worth to be evaluated?

The peer dependency approach does not sound too bad to me.

I do have a quick question: is the code in functions.ts reused more than once in some internal code? Obviously it's not used in addons-linter which is the only publicly dependent package, that's why I'm asking. If it's used only once, maybe this code could be moved to the internal project instead.

It is indeed used in different internal projects (more than one). The addons-linter might eventually rely on that functions code as well.

[rpl]

I do have a quick question: is the code in functions.ts reused more than once in some internal code? Obviously it's not used in addons-linter which is the only publicly dependent package, that's why I'm asking. If it's used only once, maybe this code could be moved to the internal project instead.

It is indeed used in different internal projects (more than one). The addons-linter might eventually rely on that functions code as well.

@willdurand May I ask you to add some additional details about this part of your last comment? (The one about addons-linter eventually rely on the functions module, I'm curious about what the addons-linter would rely on from that module in the future plans).

[willdurand]

@willdurand May I ask you to add some additional details about this part of your last comment? (The one about addons-linter eventually rely on the functions module, I'm curious about what the addons-linter would rely on from that module in the future plans).

Running addons-linter in AWS Lambda as opposed to a celery worker for AMO.

[rpl]

@willdurand May I ask you to add some additional details about this part of your last comment? (The one about addons-linter eventually rely on the functions module, I'm curious about what the addons-linter would rely on from that module in the future plans).

Running addons-linter in AWS Lambda as opposed to a celery worker for AMO.

ah, that was actually what I was guessing that functions.js module was related to.

ok, then, that sounds like something we should do as part of npm package separate from the addons-linter one, so that dependents npm packages like web-ext that do not need that wouldn't need to pull express just to be able to import the addons linting module.

[willdurand]

Alrighty, so we released a new major version (4.0.0) of this lib to fix this issue: https://github.com/mozilla/addons-scanner-utils/releases/tag/4.0.0. This new version is now used in addons-linter >= 2.13.1: https://github.com/mozilla/addons-linter/releases/tag/2.13.1

The problem in web-ext will be fully fixed once mozilla/web-ext#2081 is merged and a new version is released.