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

fix: clean overrides for removed dependencies #13170

Merged
merged 2 commits into from Feb 28, 2022

Conversation

caalador
Copy link
Contributor

Overrides should also automatically
be cleaned from the package.json
if there is no dependency that the
override would be targeting.

Overrides should also automatically
be cleaned from the package.json
if there is no dependency that the
override would be targeting.
@github-actions
Copy link

github-actions bot commented Feb 28, 2022

Unit Test Results

   943 files  +  6     943 suites  +6   41m 35s ⏱️ -26s
6 123 tests +10  6 074 ✔️ +11  49 💤 +1  0 ±0 
6 323 runs  +16  6 268 ✔️ +17  55 💤 +1  0 ±0 

Results for commit ebc775d. ± Comparison against base commit 398e07a.

♻️ This comment has been updated with latest results.

if (!(dependencies.hasKey(dependency)
|| devDependencies.hasKey(dependency))
&& overridesSection.getString(dependency).startsWith("$")) {
overridesSection.remove(dependency);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it mean that any $-overrides will be removed, even some added by the user?

Copy link
Contributor

Choose a reason for hiding this comment

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

(Note: certainly not blocking, as this may not be a real issue as this mechanism is unlikely to be used much in Flow projects, just asking for the record)

Copy link
Member

Choose a reason for hiding this comment

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

This checks the dependencies and devDependencies sections, right? So if I have a dependency on foo I can define an override for foo without this removing it?

Copy link
Contributor

Choose a reason for hiding this comment

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

So it will remove overrides starting with $ that are not in dependencies or devDependencies. As such a configuration would cause npm to raise an error anyway this is probably unlikely to be an issue ever.

@sonarcloud
Copy link

sonarcloud bot commented Feb 28, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@joheriks joheriks merged commit 5b635a4 into master Feb 28, 2022
@joheriks joheriks deleted the issues/clean-overrides-section branch February 28, 2022 13:59
vaadin-bot pushed a commit that referenced this pull request Feb 28, 2022
Overrides should also automatically
be cleaned from the package.json
if there is no dependency that the
override would be targeting.
joheriks pushed a commit that referenced this pull request Feb 28, 2022
Overrides should also automatically
be cleaned from the package.json
if there is no dependency that the
override would be targeting.

Co-authored-by: caalador <mikael.grankvist@vaadin.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants