ni / javascript-styleguide

JavaScript and TypeScript Style Guide
MIT License
9 stars 9 forks source link

Upgrade production dependencies with Angular 14 support. #82

Closed TrevorKarjanis closed 2 years ago

TrevorKarjanis commented 2 years ago

This PR tries not to break downstream consumers by introducing changes to the rule set.

Notes

Linted and tested

jattasNI commented 2 years ago

@TrevorKarjanis is there anything blocking us from taking this PR out of Draft state and getting it reviewed/submitted?

TrevorKarjanis commented 2 years ago

@TrevorKarjanis is there anything blocking us from taking this PR out of Draft state and getting it reviewed/submitted?

The biggest pain here was identifying new rules. I did so manually at first. My next step is to use the utility Milan created to print a before and after, and compare them to verify the findings described here. We then need to decide on our policy. My thought is to disable them if they are recommended, and create an issue to evaluate all of them appropriately so that upgrade PRs aren't blocked by a potentially lengthy process. As we saw in our initial evaluation, blindly enabling them is not practical. We could also take a quick pass to identify obvious cases that align or violate existing conventions.

After that, the dependencies in the PR just need to be upgraded again. Run the tests, update the README, and post the PR.

jattasNI commented 2 years ago

Let me know if I can help in any way. This iteration our team is driving the Angular 13 upgrade in many of the apps and libs that depend on this, so getting this in will be an important step and something we'd be happy to help with.

rbell517 commented 2 years ago

Is there an update on this PR? I'm trying to apply these rules to Skyline's EndToEndTests subdir and its using ES2022 features. I can change the handful of places we use those features but if this is close then I might as well wait.

TrevorKarjanis commented 2 years ago

Is there an update on this PR? I'm trying to apply these rules to Skyline's EndToEndTests subdir and its using ES2022 features. I can change the handful of places we use those features but if this is close then I might as well wait.

You're adding linting to the end-to-end tests? You're a hero! This effort will hopefully restart next week, as myself and many involved are heavily involved in the latest customer effort. This is also a part of our larger migration to Angular 13. I would suggest regressing those features temporarily as that is likely much easier than this.

rajsite commented 2 years ago

@TrevorKarjanis FYI you will need to create a new PR based on a branch from the ni repo instead of branch from your fork to merge

TrevorKarjanis commented 2 years ago

@TrevorKarjanis FYI you will need to create a new PR based on a branch from the ni repo instead of branch from your fork to merge

I happen to be doing exactly that! Thanks.