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

Add CLI command to check for undefined classnames #223

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

teckliew
Copy link
Member

@teckliew teckliew commented Jul 18, 2020

There are undefined and global classname leaks when classnames, that are not defined in the component less modules, are used or referenced within components.

Add a private command that identifies these potential unintentional global or undefined classes.
Only works on the root package directories, but not within component directories.
TODO: Check for default classes

Enact-DCO-1.0-Signed-off-by: Teck Liew ([email protected])

@webOS101
Copy link
Contributor

This is a :neat: idea. It probably, though, should end up in an ESLint module or should use a similar method of extracting the data using AST (both for LESS and JS). Though, for now, it might just be nice to run it occasionally as-is. I worry though that it could get confused with componentCss (which we use in places), which I think would be resolved by an AST approach.

@JayCanuck
Copy link
Member

JayCanuck commented Jul 20, 2020

An AST is better, but since this is crossing between both project-wide js/ts and project-wide css/less, if it were solely an eslint rule or a csslint rule, I could realistically imagine a notable processing time , along with a handful of new dependencies. Eg. in eslint, scanning/parsing all css/less files would happen for each js file).

It's difficult since it covers strange ground. For something cross-filetype like this, conceptually it might make sense as a webpack plugin at build-time (though still would have some dev hurdles in AST form there), however the usecase for this tools is currently internally prior to framework releases (like a pseudo unit-test), not in build. So a standalone command, or possibly an internal unittest pre-run function might be ideal; open to ideas.

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.

3 participants