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
Re-use global plugins inside shareable configurations #1309
Comments
i think the behavior i would expect if i was pulling in a shareable config would be (2) since the dependencies of the config would in theory be tested to be compatible with each other and not expect others that happen to be included to accidentally override. this situation is similar to the hoisting problem that is being discussed in an rfc for eslint, so that may provide some context that could help with the decision around how this should be handled. i also wonder if the fact that |
I'm pretty sure this situation is already handled and plugins are loaded locally and if absent they are loaded globally using semantic-release/lib/plugins/utils.js Line 47 in 0e24022
|
The cwd case is handled, but semantic-release may be installed globally, not in cwd (which is likely the git root of the package). |
If semantic-release is installed globally then |
No, because pluginPaths[name] is an absolute path, IIRC (not at laptop right now) |
If you have a locally installed shareable config we expect the plugin to be locally installed. This is because we expect the shareable config module to define the dependencies it needs. |
I don't think we will modify the way we load plugin and shareable config as it's already complicated enough. Here a few reasons:
Unless I'm miss something, increasing the already complex plugin system for so few benefits doesn't seems a great trade-off. |
As a side note the plugins are installed by npm based on So no matter what we would do in the semantic-release that wouldn't prevent to install plugins twice if you depends on them both locally and globally. |
I would definitely not call this an edgy case. It makes perfect sense for a large team to have a global install of The other use case I have is bundling the shared commitlint config into the same package (because commitlint and semantic-release need to have the same commit rules). I would not underestimate the extra burden of having to manage extra dependencies across a number of repos, when bundling the logic makes sense.
Installing just the
It's a one line change, as I have demonstrated in the workaround. |
the case where i could see realistically doing this myself is where the shareable config is installed locally, but to handle this in the past, i've installed the shareable config globally. that seems like the more correct way to handle this, but i could see what i initially described as being arguably more ergonomic. not necessarily saying that's enough of a reason to support it though. |
In any case, as mentioned here changing the code in semantic-release will not solve your problem. It's not semantic-release that install the dependencies, it's npm. So if you have The workaround you proposed just influence were we load the plugins from not were npm install them. They will be installed twice and we would just load the other one. |
They will only be installed twice if they are included in The whole point I'm trying to make with this issue is that there should be no need to install them twice or include them in It's the same as if you have |
We want to have the shared config to specify their dependencies so they can choose a specific version. |
Being able to chose a specific version and being forced to chose a specific version are two different things? |
from my comment above:
this is the key piece for me, and i think what @pvdlg is trying to highlight. if the shareable config defines it's own dependencies, it is able to control which version range it is compatible with. by using versions that are not provided by the config, the risk is introduced that a breaking change in the provided plugins breaks the config. this is why my expectation is that the config always defines its own dependencies, at minimum as a peer-dependency.
this might be the disconnect? by the nature of extending a shared config, my expectation is that extends the config as a whole, including its dependencies, rather than just extending it as a config file with packages installed by other means. |
Sure, I understand this bit and it is valuable. However npm does not really help here (well, at least until v7, when it will start trying to install peerDeps again, which means that defining peerDeps will bring us back to where we are now - we'll get two copies installed) for the cases where package is installed in separate places (global vs local). As an example, hapi moved away from defining peer dependencies, and instead the plugins define the hapi range they are compatible with, while the framework enforces that. This does somewhat overlap with npm's functionality, but it does offer rigidity. Probably out of scope here, I can open a separate issue to discuss this if this is interesting.
I don't see this as mutually exclusive? You can still do that - I'm just asking provide a smart fallback to reduce friction? |
New feature motivation
At the moment, when using a shareable configuration which has a
plugins
array,semantic-release
expects the plugins to be the dependencies of the shareable configuration.If you keep
semantic-release
globally installed (or usenpx
) and the shareable configuration installed per project, you will be installing the default (npm, github, etc) plugins twice. This slows down the build/release process significantly.New feature description
I can see three ways this can work:
semantic-release
, not relative to config.semantic-release
is the "official" version, whereas the version relative to the config could be outdated/incompatible (ah, peer dep management)semantic-release
.loadPlugin
tries to resolve relative to cwd as a fallback.{ local: false }
, which would then force it to be resolved relative tosemantic-release
.New feature implementation
Depends on the path chosen.
Happy to PR if there's agreement.
Workaround
The workaround is to provide the full path to the plugin. The path can be resolved relative to the "main" module, e.g.
Not pretty, but it does work.
--
Edit: added the workaround
The text was updated successfully, but these errors were encountered: