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

Update dependency versions #169

Merged
merged 5 commits into from
Jul 24, 2024
Merged

Update dependency versions #169

merged 5 commits into from
Jul 24, 2024

Conversation

rnathuji
Copy link
Contributor

No description provided.

rnathuji added 4 commits July 23, 2024 09:31
There were some breaking rule additions in the version update from
52.0.0 to 59.0.0. This change disables most of these new rules
for the time being except for @typescript-eslint/no-import-type-side-effects.
This change is due to sass update incorporating the following
breaking change:
https://sass-lang.com/documentation/breaking-changes/mixed-decls/
@rnathuji
Copy link
Contributor Author

rnathuji commented Jul 23, 2024

@openstax/k12-devs - I'm holding off on marking this ready for review until I have the next batch of dependency update PRs ready, but I wanted to get early 👀 on a couple of things here:

  1. There were new rules added to recent versions of eslint-config-love that created lint errors. Per this commit, I opted to just turn most of them off. I think that aligns with some of the decisions we made in K12-67, but LMK what y'all think. I expect we'll see something similar in the ROPE repo.

  2. The newer versions of sass are emitting deprecation warnings due to this breaking change. I made a change to our bootstrap.scss per this commit to address the bulk of them. Note that the CSS output sha didn't change with that modification, so that should give us some confidence that there are no unexpected side effects. There are still a couple of deprecation warnings per the following, but I think we'll have to wait until they're addressed in a future version of bootstrap. LMK if that assessment seems wrong to y'all:

 DEPRECATION WARNING: Sass's behavior for declarations that appear after nested
rules will be changing to match the behavior specified by CSS in an upcoming
version. To keep the existing behavior, move the declaration above the nested
rule. To opt into the new behavior, wrap the declaration in `& {}`.

More info: https://sass-lang.com/d/mixed-decls

    ┌──> node_modules/bootstrap/scss/_reboot.scss
503 │     font-weight: $legend-font-weight;
    │     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ declaration
    ╵
    ┌──> node_modules/bootstrap/scss/vendor/_rfs.scss
136 │ ┌     @media (#{$rfs-mq-property-width}: #{$rfs-mq-value}) {
137 │ │       @content;
138 │ │     }
    │ └─── nested rule
    ╵
    node_modules/bootstrap/scss/_reboot.scss 503:3  @import
    src/styles/bootstrap.scss 135:11                @import
    src/styles/main.scss 1:9                        root stylesheet

DEPRECATION WARNING: Sass's behavior for declarations that appear after nested
rules will be changing to match the behavior specified by CSS in an upcoming
version. To keep the existing behavior, move the declaration above the nested
rule. To opt into the new behavior, wrap the declaration in `& {}`.

More info: https://sass-lang.com/d/mixed-decls

    ┌──> node_modules/bootstrap/scss/_reboot.scss
504 │     line-height: inherit;
    │     ^^^^^^^^^^^^^^^^^^^^ declaration
    ╵
    ┌──> node_modules/bootstrap/scss/vendor/_rfs.scss
136 │ ┌     @media (#{$rfs-mq-property-width}: #{$rfs-mq-value}) {
137 │ │       @content;
138 │ │     }
    │ └─── nested rule
    ╵
    node_modules/bootstrap/scss/_reboot.scss 504:3  @import
    src/styles/bootstrap.scss 135:11                @import
    src/styles/main.scss 1:9                        root stylesheet

@rnathuji rnathuji requested a review from a team July 24, 2024 15:52
@MReyna12
Copy link
Contributor

Point 1: Totally fine with turning them off at this stage of the game.

Point 2: I also agree that these deprecation warnings will have to be addressed by Bootstrap.

Copy link
Contributor

@P-Gill97 P-Gill97 left a comment

Choose a reason for hiding this comment

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

Tested on local instance. Everything seems to work!

@rnathuji rnathuji merged commit 89a5b5a into main Jul 24, 2024
2 of 3 checks passed
@rnathuji rnathuji deleted the K12-86/dependency-updates branch July 24, 2024 19:13
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.

3 participants