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

sass: First attempt to port away from SASS @import #21152

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

garrett
Copy link
Member

@garrett garrett commented Oct 22, 2024

I managed to get sass-migrator -I pkg/lib -I node_modules/ module --migrate-deps pkg/**css to work after a bit of convincing. (It ran into some loops and other issues, so I had to handle each subdirectory separately, especially files in pkg/lib/, so I did this in parts and then re-ran at a top level again, just to be sure.)

This currently does not compile and does need a lot more work. It is an autogenerated migration and has lots of issues. I peeked a little at them and do see there are definitely issues.

It's a first attempt to address #21151 and definitely still needs some work.

@garrett
Copy link
Member Author

garrett commented Oct 22, 2024

This spews a lot of errors that apparently don't seem like they can be fixed. Fun. ☹️

I tried using @use foo as *; instead of just @use (in place of @import) and it doesn't work. This should, theoretically work, but doesn't. This was the issue Katerina and I were hitting a while back, the last time we tried porting.

The SASS migration tool does not do a good job at migrating.

@martinpitt martinpitt added the no-test For doc/workflow changes, or experiments which don't need a full CI run, label Oct 22, 2024
@garrett
Copy link
Member Author

garrett commented Oct 23, 2024

Here are all the errors we get after using the SASS migration tool (note that some of these are just plain CSS that SASS decided to think are SASS, like min(); others are due to SASS imported in JSX files, and yet others are the result of @use not actually being equivalent to @import, despite what SASS docs say):

 [ERROR] 20rem and 50vh have incompatible units.
   
52    max-block-size: math.min(20rem, 50vh);
                      ^^^^^^^^^^^^^^^^^^^^^
   
  pkg/lib/patternfly/patternfly-5-overrides.scss 52:19  @use
  pkg/lib/page.scss 3:1                                 root stylesheet [plugin sass-plugin]

    pkg/playground/journal.jsx:24:7:
      24  import "page.scss";
                 ~~~~~~~~~~~

 [ERROR] 20rem and 50vh have incompatible units.
   
52    max-block-size: math.min(20rem, 50vh);
                      ^^^^^^^^^^^^^^^^^^^^^
   
  pkg/lib/patternfly/patternfly-5-overrides.scss 52:19  @use
  page.scss 3:1                                         @use
  pkg/selinux/setroubleshoot.scss 21:1                  root stylesheet [plugin sass-plugin]

    pkg/selinux/selinux.js:33:7:
      33  import "./setroubleshoot.scss";
                 ~~~~~~~~~~~~~~~~~~~~~~~

 [ERROR] 20rem and 50vh have incompatible units.
   
52    max-block-size: math.min(20rem, 50vh);
                      ^^^^^^^^^^^^^^^^^^^^^
   
  pkg/lib/patternfly/patternfly-5-overrides.scss 52:19  @use
  page.scss 3:1                                         @use
  pkg/users/users.scss 3:1                              root stylesheet [plugin sass-plugin]

    pkg/users/users.js:37:7:
      37  import "./users.scss";
                 ~~~~~~~~~~~~~~

 [ERROR] 20rem and 50vh have incompatible units.
   
52    max-block-size: math.min(20rem, 50vh);
                      ^^^^^^^^^^^^^^^^^^^^^
   
  pkg/lib/patternfly/patternfly-5-overrides.scss 52:19  @use
  page.scss 3:1                                         @use
  pkg/shell/shell.scss 22:1                             root stylesheet [plugin sass-plugin]

    pkg/shell/shell.jsx:41:7:
      41  import "./shell.scss";
                 ~~~~~~~~~~~~~~

 [ERROR] 20rem and 50vh have incompatible units.
   
52    max-block-size: math.min(20rem, 50vh);
                      ^^^^^^^^^^^^^^^^^^^^^
   
  pkg/lib/patternfly/patternfly-5-overrides.scss 52:19  @use
  page.scss 3:1                                         @use
  pkg/systemd/system-global.scss 1:1                    @use
  pkg/systemd/overview.scss 1:1                         root stylesheet [plugin sass-plugin]

    pkg/systemd/overview.jsx:44:7:
      44  import "./overview.scss";
                 ~~~~~~~~~~~~~~~~~

 [ERROR] 20rem and 50vh have incompatible units.
   
52    max-block-size: math.min(20rem, 50vh);
                      ^^^^^^^^^^^^^^^^^^^^^
   
  pkg/lib/patternfly/patternfly-5-overrides.scss 52:19  @use
  page.scss 3:1                                         @use
  pkg/storaged/storage.scss 20:1                        root stylesheet [plugin sass-plugin]

    pkg/storaged/storaged.jsx:36:7:
      36  import "./storage.scss";
                 ~~~~~~~~~~~~~~~~

 [ERROR] 20rem and 50vh have incompatible units.
   
52    max-block-size: math.min(20rem, 50vh);
                      ^^^^^^^^^^^^^^^^^^^^^
   
  pkg/lib/patternfly/patternfly-5-overrides.scss 52:19  @use
  page.scss 3:1                                         @use
  pkg/systemd/terminal.scss 2:1                         root stylesheet [plugin sass-plugin]

    pkg/systemd/terminal.jsx:11:7:
      11  import "./terminal.scss";
                 ~~~~~~~~~~~~~~~~~

 [ERROR] 20rem and 50vh have incompatible units.
   
52    max-block-size: math.min(20rem, 50vh);
                      ^^^^^^^^^^^^^^^^^^^^^
   
  pkg/lib/patternfly/patternfly-5-overrides.scss 52:19  @use
  page.scss 3:1                                         @use
  pkg/kdump/kdump.scss 20:1                             root stylesheet [plugin sass-plugin]

    pkg/kdump/kdump.js:34:7:
      34  import './kdump.scss';
                 ~~~~~~~~~~~~~~

 [ERROR] 20rem and 50vh have incompatible units.
   
52    max-block-size: math.min(20rem, 50vh);
                      ^^^^^^^^^^^^^^^^^^^^^
   
  pkg/lib/patternfly/patternfly-5-overrides.scss 52:19  @use
  page.scss 3:1                                         @use
  pkg/systemd/system-global.scss 1:1                    @use
  pkg/systemd/services.scss 1:1                         root stylesheet [plugin sass-plugin]

    pkg/systemd/services.jsx:49:7:
      49  import "./services.scss";
                 ~~~~~~~~~~~~~~~~~

 [ERROR] 20rem and 50vh have incompatible units.
   
52    max-block-size: math.min(20rem, 50vh);
                      ^^^^^^^^^^^^^^^^^^^^^
   
  pkg/lib/patternfly/patternfly-5-overrides.scss 52:19  @use
  page.scss 3:1                                         @use
  pkg/systemd/system-global.scss 1:1                    @use
  pkg/systemd/logs.scss 3:1                             root stylesheet [plugin sass-plugin]

    pkg/systemd/logs.jsx:52:7:
      52  import "./logs.scss";
                 ~~~~~~~~~~~~~

 [ERROR] 20rem and 50vh have incompatible units.
   
52    max-block-size: math.min(20rem, 50vh);
                      ^^^^^^^^^^^^^^^^^^^^^
   
  pkg/lib/patternfly/patternfly-5-overrides.scss 52:19  @use
  page.scss 3:1                                         @use
  pkg/metrics/metrics.scss 1:1                          root stylesheet [plugin sass-plugin]

    pkg/metrics/metrics.jsx:64:7:
      64  import "./metrics.scss";
                 ~~~~~~~~~~~~~~~~

 [ERROR] 20rem and 50vh have incompatible units.
   
52    max-block-size: math.min(20rem, 50vh);
                      ^^^^^^^^^^^^^^^^^^^^^
   
  pkg/lib/patternfly/patternfly-5-overrides.scss 52:19  @use
  page.scss 3:1                                         @use
  pkg/sosreport/sosreport.scss 1:1                      root stylesheet [plugin sass-plugin]

    pkg/sosreport/sosreport.jsx:60:7:
      60  import './sosreport.scss';
                 ~~~~~~~~~~~~~~~~~~

 [ERROR] 20rem and 50vh have incompatible units.
   
52    max-block-size: math.min(20rem, 50vh);
                      ^^^^^^^^^^^^^^^^^^^^^
   
  pkg/lib/patternfly/patternfly-5-overrides.scss 52:19  @use
  page.scss 3:1                                         @use
  pkg/networkmanager/networking.scss 2:1                root stylesheet [plugin sass-plugin]

    pkg/networkmanager/firewall.jsx:55:7:
      55  import "./networking.scss";
                 ~~~~~~~~~~~~~~~~~~~

 [ERROR] 20rem and 50vh have incompatible units.
   
52    max-block-size: math.min(20rem, 50vh);
                      ^^^^^^^^^^^^^^^^^^^^^
   
  pkg/lib/patternfly/patternfly-5-overrides.scss 52:19  @use
  page.scss 3:1                                         @use
  pkg/systemd/hwinfo.scss 2:1                           root stylesheet [plugin sass-plugin]

    pkg/systemd/hwinfo.jsx:55:7:
      55  import "./hwinfo.scss";
                 ~~~~~~~~~~~~~~~

 [ERROR] 20rem and 50vh have incompatible units.
   
52    max-block-size: math.min(20rem, 50vh);
                      ^^^^^^^^^^^^^^^^^^^^^
   
  pkg/lib/patternfly/patternfly-5-overrides.scss 52:19  @use
  page.scss 3:1                                         @use
  pkg/packagekit/updates.scss 2:1                       root stylesheet [plugin sass-plugin]

    pkg/packagekit/updates.jsx:79:7:
      79  import "./updates.scss";
                 ~~~~~~~~~~~~~~~~

 [WARNING] Sass @import rules are deprecated and will be removed in Dart Sass 3.0.0.

More info and automated migrator: https://sass-lang.com/d/import [plugin sass-plugin]

    node_modules/@patternfly/patternfly/base/_chart-globals.scss:0:8:
      0  "../sass-utilities/colors"
                 ^

  The plugin "sass-plugin" was triggered by this import

    pkg/lib/cockpit-components-plot.jsx:33:7:
      33  import '@patternfly/patternfly/patternfly-charts.scss';
                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 [WARNING] Sass @import rules are deprecated and will be removed in Dart Sass 3.0.0.

More info and automated migrator: https://sass-lang.com/d/import [plugin sass-plugin]

    node_modules/@patternfly/patternfly/base/_chart-globals.scss:1:8:
      1  "../sass-utilities/scss-variables"
                 ^

  The plugin "sass-plugin" was triggered by this import

    pkg/lib/cockpit-components-plot.jsx:33:7:
      33  import '@patternfly/patternfly/patternfly-charts.scss';
                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 [WARNING] Sass @import rules are deprecated and will be removed in Dart Sass 3.0.0.

More info and automated migrator: https://sass-lang.com/d/import [plugin sass-plugin]

    node_modules/@patternfly/patternfly/patternfly-charts.scss:0:8:
      0  "base/chart-globals"
                 ^

  The plugin "sass-plugin" was triggered by this import

    pkg/lib/cockpit-components-plot.jsx:33:7:
      33  import '@patternfly/patternfly/patternfly-charts.scss';
                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 [WARNING] Sass @import rules are deprecated and will be removed in Dart Sass 3.0.0.

More info and automated migrator: https://sass-lang.com/d/import [plugin sass-plugin]

    node_modules/@patternfly/patternfly/sass-utilities/scss-variables.scss:0:8:
      0  './init'
                 ^

  The plugin "sass-plugin" was triggered by this import

    pkg/lib/cockpit-components-plot.jsx:33:7:
      33  import '@patternfly/patternfly/patternfly-charts.scss';
                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 [ERROR] 20rem and 50vh have incompatible units.
   
52    max-block-size: math.min(20rem, 50vh);
                      ^^^^^^^^^^^^^^^^^^^^^
   
  pkg/lib/patternfly/patternfly-5-overrides.scss 52:19  @use
  page.scss 3:1                                         @use
  pkg/apps/application.scss 1:1                         root stylesheet [plugin sass-plugin]

    pkg/apps/application.jsx:34:7:
      34  import "./application.scss";
                 ~~~~~~~~~~~~~~~~~~~~

 [ERROR] Undefined variable.
  
6      font-family var(--#{$pf-global}--FontFamily--text)
                           ^^^^^^^^^^
  
  @patternfly/patternfly/utilities/Text/text.scss 6:25  @use
  pkg/systemd/overview-cards/lastLogin.scss 2:1         root stylesheet [plugin sass-plugin]

    pkg/systemd/overview-cards/lastLogin.jsx:28:7:
      28  import './lastLogin.scss';
                 ~~~~~~~~~~~~~~~~~~

 [ERROR] Undefined variable.
  
6      font-family var(--#{$pf-global}--FontFamily--text)
                           ^^^^^^^^^^
  
  @patternfly/patternfly/utilities/Text/text.scss 6:25  @use
  pkg/lib/cockpit-components-password.scss 2:1          root stylesheet [plugin sass-plugin]

    pkg/lib/cockpit-components-password.jsx:34:7:
      34  import './cockpit-components-password.scss';
                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 [ERROR] Undefined variable.
  
6      font-family var(--#{$pf-global}--FontFamily--text)
                           ^^^^^^^^^^
  
  @patternfly/patternfly/utilities/Text/text.scss 6:25  @use
  pkg/networkmanager/wireguard.scss 2:1                 root stylesheet [plugin sass-plugin]

    pkg/networkmanager/wireguard.jsx:40:7:
      40  import './wireguard.scss';
                 ~~~~~~~~~~~~~~~~~~

11:51:11: Build finished

@martinpitt
Copy link
Member

Thanks @garrett -- but as discussed let's revert (#21155), and shelve this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-test For doc/workflow changes, or experiments which don't need a full CI run,
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants