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.

Sign up for GitHub

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

Jump to bottom

feat(svelte): option to warn on unused classes #703

Closed
wants to merge 22 commits into from
Closed

feat(svelte): option to warn on unused classes #703

wants to merge 22 commits into from

Conversation

tivac
Copy link
Owner

@tivac tivac commented Dec 17, 2019 •
edited
Loading

Description

This adds a new option to @modular-css/svelte, warnOnUnused. When set to true this will cause warnings to be issued for all the classes within the processed <link> or <style> that don't appear somewhere in the template code.

Motivation and Context

Fixes #702

How Has This Been Tested?

Added tests in the PR.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@netlify
Copy link

netlify bot commented Dec 17, 2019 •
edited
Loading

Deploy preview for m-css failed.

Built with commit ccfce2b

https://app.netlify.com/sites/m-css/deploys/5e1186e591098d0008699b22

@tivac tivac marked this pull request as ready for review December 17, 2019 08:11
@codecov
Copy link

codecov bot commented Dec 17, 2019 •
edited
Loading

Codecov Report

Merging #703 into master will decrease coverage by 0.25%.
The diff coverage is 95.69%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #703      +/-   ##
=========================================
- Coverage   98.45%   98.2%   -0.26%     
=========================================
  Files          45      46       +1     
  Lines        1168    1224      +56     
  Branches      181     190       +9     
=========================================
+ Hits         1150    1202      +52     
- Misses         15      19       +4     
  Partials        3       3
Impacted Files Coverage Δ
packages/processor/plugins/composition.js 100% <100%> (ø) ⬆️
packages/processor/lib/deferred.js 100% <100%> (ø)
packages/test-utils/relative.js 100% <100%> (ø) ⬆️
packages/processor/processor.js 100% <100%> (ø) ⬆️
packages/svelte/svelte.js 100% <100%> (ø) ⬆️
packages/processor/lib/identifiers.js 100% <100%> (ø) ⬆️
packages/rollup/rollup.js 96.55% <33.33%> (-3.45%) ⬇️

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 c6ebf81...a3a0a57. Read the comment docs.

@tivac
Copy link
Owner Author

tivac commented Dec 17, 2019

I got a chance to run this on a real codebase this morning and... yikes.

It's way, way too naive to actually use in this form.

Probably needs to be implemented as a method on the Processor instance instead of solely living inside the svelte preprocessor. That way it can take a more holistic view once everything is said & done, the per-file approach required by the limitations of the svelte preprocessor is just not at all up to the task of providing useful data.

@tivac
Copy link
Owner Author

tivac commented Dec 19, 2019

This version might be better, it's certainly the "more holistic" approach I mentioned previously. Will run it on real code tomorrow and see how scary things look.

@tivac
WIP: rework composition depgraph
93d776b 
Mostly so it's more useful in the processor
@tivac
WIP: minor reformat
83e8435
@tivac
WIP rework unused tracking
e172545 
Walk each selector's depgraph now to ensure dependencies are marked as used correctly.

Also adjusted invalidation logic so reprocessing only happens if the file is invalidated AND the contents changed.
@tivac
WIP: only output once per build
62665b0 
Not once per bundle, yikes
@tivac
WIP: lint touch-ups
544ed23
@tivac
WIP: throw at files that use @COMPOSES
887495c 
Not even gonna bother with that right now.
@tivac
WIP: more legibile output
6898182 
Still kinda basic, but better.
@tivac
WIP: remove svelte tests
7e86fb6 
Moved functionality into processor
@tivac
WIP: extract out dep walking
baf1499 
Also sneak in a deferred, my favorite.
@tivac
WIP: rework file ingestion
06863d1 
Simplifies things a little bit
@tivac
WIP: split apart API tests
d8177d4 
To help keep me more sane, in theory!
@tivac
WIP: simple test for unused()
2e612de
@tivac
WIP: add a todo test for later me
e237069
@tivac
WIP: adjust snapshot paths
de4b277 
Also make relative helper accept single files
@tivac
WIP: improve coverage
4f36c96
@tivac
WIP: updated snapshot
ccfce2b
@tivac
chore: roll back markdown-it, AGAIN
a3a0a57
@tivac
Copy link
Owner Author

tivac commented Jan 5, 2020

Still need to add that missing rollup test, and probably docs.

@ghost ghost changed the base branch from master to main June 29, 2020 03:44
@tivac
Copy link
Owner Author

tivac commented Feb 3, 2022

Too far out of date to be useful, I suspect.

@tivac tivac closed this Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add warnings for unused class
1 participant
@tivac