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

chore: replace optimize-css-assets-webpack-plugin with css-minimizer-webpack-plugin #2525

Merged
merged 6 commits into from
Jun 7, 2022

Conversation

roaga
Copy link
Contributor

@roaga roaga commented Jun 1, 2022

Related to issue #2495

Reviewers

@seve @Bento007


Changes

Replaced occurrence of optimize-css-assets-webpack-plugin with css-minimizer-webpack-plugin and updated dependencies.

@seve
Copy link
Member

seve commented Jun 1, 2022

Solid! Can we also remove clean-css from our dependencies now that we aren't calling it?

Also you might need to change the title of the PR to match conventional commits

@codecov
Copy link

codecov bot commented Jun 1, 2022

Codecov Report

Merging #2525 (6613f22) into main (f2bd6eb) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head 6613f22 differs from pull request most recent head 4938e65. Consider uploading reports for the commit 4938e65 to get more accurate results

@@            Coverage Diff             @@
##             main    #2525      +/-   ##
==========================================
- Coverage   71.64%   71.61%   -0.03%     
==========================================
  Files          94       94              
  Lines        6464     6496      +32     
  Branches      738      770      +32     
==========================================
+ Hits         4631     4652      +21     
- Misses       1757     1768      +11     
  Partials       76       76              
Flag Coverage Δ
frontend 71.61% <100.00%> (-0.03%) ⬇️
javascript 71.61% <100.00%> (-0.03%) ⬇️
unitTest 71.61% <100.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
client/src/annoMatrix/viewCreators.js 73.91% <100.00%> (+1.18%) ⬆️
client/src/annoMatrix/whereCache.js 100.00% <100.00%> (ø)
client/src/util/typedCrossfilter/crossfilter.js 92.61% <0.00%> (-1.12%) ⬇️
client/src/util/actionHelpers.js 76.27% <0.00%> (-0.93%) ⬇️
client/src/util/typedCrossfilter/bitArray.js 77.18% <0.00%> (-0.91%) ⬇️
client/src/util/stateManager/matrix.js 85.84% <0.00%> (-0.65%) ⬇️
client/src/util/typedCrossfilter/sort.js 89.17% <0.00%> (-0.51%) ⬇️
client/src/util/stateManager/colorHelpers.js 47.47% <0.00%> (-0.49%) ⬇️
...ent/__tests__/util/stateManager/sampleResponses.js 100.00% <0.00%> (ø)
client/src/util/dataframe/dataframe.js 93.30% <0.00%> (+0.07%) ⬆️
... and 4 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 f2bd6eb...4938e65. Read the comment docs.

@roaga roaga changed the title Replace optimize-css-assets-webpack-plugin with css-minimizer-webpack… chore: replace optimize-css-assets-webpack-plugin with css-minimizer-webpack-plugin Jun 1, 2022
@roaga
Copy link
Contributor Author

roaga commented Jun 1, 2022

I fixed the PR title.
It seems that clean-css is still required by the package html-minifier-terser, which is required by html-webpack-plugin, which is still used.

@seve seve self-requested a review June 3, 2022 18:50
@seve seve requested a review from tihuan June 3, 2022 20:07
@seve
Copy link
Member

seve commented Jun 3, 2022

@tihuan Do these snapshot changes make sense to you?

@tihuan
Copy link
Contributor

tihuan commented Jun 3, 2022

Thanks for pinging!

Huh, the inline style is completely gone in the snapshot. Does the app look good running locally?

UPDATE: Oh wait, my bad! It's just Github's weird syntax highlighting 😆

Screen Shot 2022-06-03 at 2 04 09 PM

It looks like most of the snapshot diffs are adding aria-hidden=\\"true\\". And the following CSS class name did change from categorical__value___2m6V7 to categorical__value___wHOeo, which is probably okay, because we changed the plugin 👌

If the app looks fine running locally, then LGTM!

Screen Shot 2022-06-03 at 2 09 11 PM

@roaga roaga enabled auto-merge (squash) June 7, 2022 20:02
@Bento007 Bento007 disabled auto-merge June 7, 2022 20:05
@Bento007 Bento007 enabled auto-merge (squash) June 7, 2022 20:05
@Bento007 Bento007 disabled auto-merge June 7, 2022 20:08
@Bento007 Bento007 enabled auto-merge (squash) June 7, 2022 20:09
@Bento007 Bento007 disabled auto-merge June 7, 2022 20:13
@Bento007 Bento007 merged commit 8b4c1e4 into main Jun 7, 2022
@Bento007 Bento007 deleted the roaga/css-minimizer branch June 7, 2022 20:13
ebezzi added a commit that referenced this pull request Jul 11, 2022
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.

None yet

4 participants