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

new purgecss-from-jsx plugin #692

Merged
merged 3 commits into from
Jul 6, 2021
Merged

new purgecss-from-jsx plugin #692

merged 3 commits into from
Jul 6, 2021

Conversation

Nauja
Copy link
Contributor

@Nauja Nauja commented Jun 4, 2021

Proposed changes

I wrote a new plugin for purgecss to extract id, class and tag attributes from JSX files.

Types of changes

  • Bugfix (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 not work as expected)

Checklist

  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

You will find a working test in purgecss-from-jsx/__tests__.

I added acorn acorn-walk acorn-jsx and acorn-jsx-walk dependencies to code this plugin and I also added necessary .d.ts files in types directory.

For now it only support simple literal attributes for id, className and tag name, meaning it won't evaluate a JSX expression to a string and extract ids, classes or tags from it.

But this is sufficient for a simple JSX file as:

import React from "react";

class MyComponent extends React.Component {
  render() {
    return (
      <React.Fragment>
        <div className="test-container">Well</div>
        <div className="test-footer" id="an-id"></div>
        <a href="#" id="a-link" className="a-link"></a>
        <input id="blo" type="text" disabled/>
      </React.Fragment>
    );
  }
}

export default MyComponent;

Hope you enjoy it :)

@Ffloriel
Copy link
Member

Thanks!! That's great! I'll take a look at this PR this week

@Ffloriel
Copy link
Member

@Nauja Looks good to me! I can see the types for acorn are not precise and you had to use any quite often. I looked around and saw some libraries added their own .d.ts too, but with more accurate types. It would be worth getting more accurate types but that can be done at a later time.

For now it only support simple literal attributes for id, className and tag name, meaning it won't evaluate a JSX expression to a string and extract ids, classes or tags from it.

Are you thinking of adding support for cases like in the example below in this PR, or a following? If not in this PR, I will approve and merge this one. Let me know.

import React from "react";

fonction getHello() {
    return "class-hello" + " second-class"
}

const HELLO = getHello()

class MyComponent extends React.Component {
  render() {
    return (
      <React.Fragment>
        <div className={HELLO}>Well</div>
        <div className="test-footer" id="an-id"></div>
        <a href="#" id="a-link" className="a-link"></a>
        <input id="blo" type="text" disabled/>
      </React.Fragment>
    );
  }
}

export default MyComponent;

@Nauja
Copy link
Contributor Author

Nauja commented Jun 21, 2021

I looked around and saw some libraries added their own .d.ts too, but with more accurate types

What libraries did you find ? Maybe I can copy their type declarations and remove all those any.

Are you thinking of adding support for cases like in the example below in this PR, or a following? If not in this PR, I will approve and merge this one. Let me know.

Would be nice. For now I coded this plugin for a personal project and I didn't encounter the case where className is a JSX expression. I don't know how it would work evaluating such expression as it could include code from another module or an object created at runtime:

<div className={`total total-${obj.state}`}>...</div>

I think it would be a nice but complex feature to have in a next PR.

@Ffloriel
Copy link
Member

@Nauja
Copy link
Contributor Author

Nauja commented Jun 29, 2021

From what I tested, it's impossible to do:

        JSXOpeningElement(node: JSXOpeningElement, state: any, callback) {
          // JSXIdentifier | JSXMemberExpression | JSXNamespacedName
          const nameState: any = {};
          callback(node.name, nameState);

Because acorn.Node is not assignable to JSXOpeningElement.

But it's possible to do:

        JSXOpeningElement(node: unknown, state: any, callback) {
          const n = node as JSXOpeningElement;
          // JSXIdentifier | JSXMemberExpression | JSXNamespacedName
          const nameState: any = {};
          callback(n.name, nameState);

However, I don't know if it's worth adding noise and unknown in the code to avoid any in some private code, especially when picking a .d.ts file from a random source.

I will do another commit for this PR because acorn-walk got a new version with a fix I did in their .d.ts

@Nauja
Copy link
Contributor Author

Nauja commented Jun 29, 2021

So I updated acorn-walk version and removed acorn-walk.d.ts as the thing I added is now included in the package.

I searched why the checks failed but I see it's not related to this commit.

@Ffloriel Ffloriel merged commit 3570c7d into FullHuman:master Jul 6, 2021
@Ffloriel
Copy link
Member

Ffloriel commented Jul 6, 2021

Merged! Thanks for this new plugin. It will be published with the next version

@Nauja
Copy link
Contributor Author

Nauja commented Jul 7, 2021

Cool, thanks :)

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.

2 participants