-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
build: dedupe theming scss bundle #4174
Conversation
tools/gulp/tasks/release.ts
Outdated
@@ -32,6 +27,10 @@ const themingEntryPointPath = join(COMPONENTS_DIR, 'core', 'theming', '_all-them | |||
// Output path for the scss theming bundle. | |||
const themingBundlePath = join(releasePath, '_theming.scss'); | |||
|
|||
// Glob that matches all files that might be imported multiple times. | |||
// Necessary for deduping inside of scss-bundle. | |||
const themingBundleDedupeGlob = join(COMPONENTS_DIR, '**/*.scss'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be named for what is it rather than how it's used, e.g., allScssGlob
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I named it themingBundleDedupeGlob
because I wanted to make it clear that this will be the array that holds the files that might need deduping. Changed it because the comment below explains it then.
tools/gulp/tasks/release.ts
Outdated
], {silentStdout: true} | ||
)); | ||
task(':bundle:theming-scss', () => { | ||
new Bundler().Bundle(themingEntryPointPath, [themingBundleDedupeGlob]).then(result => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you comment on what the arguments are here since it's not really clear from the call site?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
* Updates to a more recent version of SCSS-bundle that has support for deduping. The dedupe process is very naive and only disallows importing a file multiple times. * Switches to the programmatic API of `scss-bundle` * Removes unused functions and imports / leftovers. Fixes angular#3931
1fc9a3a
to
d364b6e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
scss-bundle
Fixes #3931