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

refactor: migrate to the Sass module system #5453

Merged
merged 2 commits into from Jan 14, 2020

Conversation

@jathak
Copy link
Contributor

jathak commented Jan 10, 2020

This PR was generated by running this script based on the Sass migrator. It migrates all components 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).

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 files via @use, only members directly declared in that file are available. The mdc-$component.scss files should only expose CSS, not Sass members.
  • 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.
  • Minor breaking change: Four variables and a mixin in mdc-textfield were renamed to use a mdc-text-field- prefix when depended on via @import. None of the variables are used within Google and the mixin (formerly mdc-required-text-field-label-asterisk_, now required-label-asterisk_) is used only once by MWC.
@googlebot googlebot added the cla: yes label Jan 10, 2020
@jathak

This comment has been minimized.

Copy link
Contributor Author

jathak commented Jan 10, 2020

One other note about the script used here: while the underlying Sass migrator is fairly robust, the script itself is fragile, as preserving the proper prefixes for some of the more complex components (e.g. mdc-ripple, mdc-select, mdc-textfield) required a bunch of sed transformations, some of which reference specific lines to change. We can reuse the script if there are enough changes to master before this is submitted to make a merge difficult, but future runs may require tweaks to it.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 10, 2020

Codecov Report

Merging #5453 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5453      +/-   ##
==========================================
- Coverage   97.06%   97.04%   -0.02%     
==========================================
  Files         164      164              
  Lines        6266     6266              
  Branches      824      782      -42     
==========================================
- Hits         6082     6081       -1     
- Misses        184      185       +1
Impacted Files Coverage Δ
testing/dom/events.ts 71.42% <0%> (-28.58%) ⬇️
packages/mdc-auto-init/index.ts 96% <0%> (+4%) ⬆️

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 981ec9b...a19bf66. Read the comment docs.

@jathak jathak force-pushed the jathak:module-system-migration branch 2 times, most recently from 62c6d7c to 5593f9f Jan 10, 2020
@jathak

This comment has been minimized.

Copy link
Contributor Author

jathak commented Jan 10, 2020

Okay, the lint issue was that stylelint-scss was out of date. The unit tests seem to be failing because of an issue with Chrome

@jathak

This comment has been minimized.

Copy link
Contributor Author

jathak commented Jan 11, 2020

@jathak jathak force-pushed the jathak:module-system-migration branch from 4142d1a to a19bf66 Jan 13, 2020
Copy link
Contributor

abhiomkar left a comment

🎉

@abhiomkar abhiomkar merged commit faa9af3 into material-components:master Jan 14, 2020
6 of 7 checks passed
6 of 7 checks passed
lint
Details
build-test
Details
site-generator-test
Details
unit-test unit-test
Details
Semantic Pull Request ready to be squashed
Details
Travis CI - Pull Request Build Passed
Details
cla/google All necessary CLAs are signed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.