Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

refactor: migrate to Sass module system (wip) #5354

Closed
wants to merge 6 commits into from

Conversation

jathak
Copy link
Contributor

@jathak jathak commented Dec 18, 2019

DO NOT MERGE. This PR is just designed to demonstrate the result of the migration script and discuss the changes that are necessary.

This PR was generated by running this script on top of #5337. It migrates most components (excluding chips, select, and textfield) to the Sass module system, removing manual mdc-$component prefixes and implicit dependencies between files (though still leaving both intact for users still using @import).

The Sass migrator focuses on correctness and the API that each files exposes, so the code it outputs may not always conform to your preferred style. In most cases, those style changes should be applied on top of the migrator's output, but if there's something style-related that you think it makes sense for the migrator to handle, we could add that support.

Changes to the general structure of the code after migration will require changes to the script, which should then be re-run on top of master.

High-level overview of the changes made here:

  • Prefixes (e.g. mdc-button-) for each component have been removed from all variables, functions, and mixins within the code. Prefixes on CSS selectors are unaffected.
  • When depending on one of a component's partial files via @use, only members directly declared in that file are available.
  • The non-partial files for components that have them do forward members from their own partials, but not from other components (so mdc-button.scss exposes the button mixins, but not the theme mixins, for example)
  • Import-only files have been generated that add back all of the prefixes and explicitly forward all members that were implicitly available via @import previously, so existing users that still use @import shouldn't break as a result of these changes. These files will then be used by the migrator when those users later migrate to the module system themselves.

@jathak jathak force-pushed the migration-demo branch 2 times, most recently from 5bdeacc to 3015dee Compare December 20, 2019 19:00
@codecov-io
Copy link

codecov-io commented Dec 20, 2019

Codecov Report

Merging #5354 into master will increase coverage by 0.97%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5354      +/-   ##
==========================================
+ Coverage   96.34%   97.31%   +0.97%     
==========================================
  Files         163      164       +1     
  Lines        6260     6257       -3     
  Branches      834      781      -53     
==========================================
+ Hits         6031     6089      +58     
+ Misses        229      168      -61
Impacted Files Coverage Δ
packages/mdc-textfield/constants.ts 100% <ø> (ø) ⬆️
packages/mdc-textfield/component.ts 97.95% <ø> (-0.07%) ⬇️
packages/mdc-switch/component.ts 30.35% <0%> (-61.17%) ⬇️
packages/mdc-notched-outline/component.ts 60.71% <0%> (-35.84%) ⬇️
testing/dom/events.ts 71.42% <0%> (-28.58%) ⬇️
packages/mdc-switch/foundation.ts 97.29% <0%> (-2.71%) ⬇️
packages/mdc-line-ripple/foundation.ts 100% <0%> (ø) ⬆️
packages/mdc-floating-label/component.ts 100% <0%> (ø) ⬆️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ac72d7...2792051. Read the comment docs.

@jathak jathak force-pushed the migration-demo branch 2 times, most recently from 7bd4019 to 440ef0d Compare December 20, 2019 19:53
@jathak jathak force-pushed the migration-demo branch 3 times, most recently from b2bc76a to b00d0a1 Compare January 3, 2020 18:24
@jathak jathak force-pushed the migration-demo branch 2 times, most recently from 62a693e to 50046ab Compare January 6, 2020 18:46
jathak added 2 commits January 6, 2020 13:47
This replaces `__` and `--` in certain mixin names with a single dash to
allow migrating to the module system without breaking downstream users
that use these pseudoprivate mixins.
@jathak jathak force-pushed the migration-demo branch 2 times, most recently from 9fdecc4 to 4a9e64b Compare January 9, 2020 00:03
@jathak jathak force-pushed the migration-demo branch 4 times, most recently from 700c773 to db434ef Compare January 9, 2020 02:36
@jathak jathak closed this Jan 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants