Skip to content

chore: don't create separate renovate PRs for angular and angular-cli #290

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

Merged
merged 1 commit into from
Apr 11, 2025

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Apr 10, 2025

This groups all used angular packages in single PR instead of separate for CLI and angular/core repo which just won't work when upgrading separately.

pieh#3 is example produced angular packages bump with proposed configuration change

Additionally this disables major bumps completely - while ideally we would have one (but just for demo app, because we do want to keep fixtures as-is), it doesn't seem feasible to configure renovate to make all required bumps - my attempt was failing with following npm error:

npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR! 
npm ERR! While resolving: [email protected]
npm ERR! Found: [email protected]
npm ERR! node_modules/typescript
npm ERR!   dev typescript@"~5.6.2" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer typescript@">=5.8 <5.9" from @angular/compiler-cli@[20](https://github.com/pieh/angular-runtime/actions/runs/14374559652/job/40303889258?pr=4#step:6:21).0.0-next.6
npm ERR! node_modules/@angular/compiler-cli
npm ERR!   dev @angular/compiler-cli@"^20.0.0-next" from the root project
npm ERR!   peer @angular/compiler-cli@"^20.0.0 || ^20.0.0-next.0" from @angular-devkit/[email protected]
npm ERR!   node_modules/@angular-devkit/build-angular
npm ERR!     dev @angular-devkit/build-angular@"^20.0.0-next" from the root project

( pieh#4 for example)

so at least cases like this with peerDependencies would somehow be handled - this just disables major bumps completely

@pieh pieh requested a review from a team as a code owner April 10, 2025 07:40
Copy link

netlify bot commented Apr 10, 2025

Deploy Preview for plugin-angular-universal-demo ready!

Name Link
🔨 Latest commit
🔍 Latest deploy log https://app.netlify.com/sites/plugin-angular-universal-demo/deploys/67f793161a8b1a21ab1a1061
😎 Deploy Preview https://deploy-preview-290--plugin-angular-universal-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@serhalp
Copy link
Contributor

serhalp commented Apr 10, 2025

Ah, right. So ideally we would use postUpgradeCommands that use ng update to handle things like typescript peer deps, but since we're on the GitHub App version of Renovate, only certain postUpgradeCommands are allowed: https://docs.renovatebot.com/mend-hosted/hosted-apps-config/#allowed-post-upgrade-commands. I checked in our logs and it's these:

    "allowedCommands": [
      "^git add --all$",
      "^git reset$",
      "^pwd$"
    ],

So, this isn't an option at the moment.

I wonder if we should add typescript to the angular group?

Copy link
Contributor

@serhalp serhalp left a comment

Choose a reason for hiding this comment

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

LGTM, thank your for digging into this!

matchFileNames: ['tests/**/fixtures/**/package.json', 'demo/package.json'],
matchPackageNames: ['@angular/**', '@angular-devkit/**'],
matchUpdateTypes: ['major'],
enabled: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could just not set the bump-framework-in-fixtures label for these instead of disabling entirely? We wouldn't get the noisy failure notifications but we'd still have a little bit of visibility into available major bumps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we could make that work, then it would be great!

Some troubles I did have with angular setup specifically is that "breaking changes" is mix of major and minor (0.X range for zone.js) which afaik is really painful to handle with renovate. Maybe there is some nicer setting than updateType (major/minor/patch) to play with. I'm not actually sure how to configure not allowing minor bumps for zone.js for non-breaking changes PRs while allowing this for breaking changes PRs. And then conditionally applying that label

In any case - I'll merge this as-is to get currently supported majors to receive correct non-breaking version bumps and we can look into making major version bumps as follow up

@pieh pieh merged commit f0ce7ad into netlify:main Apr 11, 2025
8 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants