WPTT / theme-sniffer

Theme Sniffer plugin using sniffs.
MIT License
269 stars 3 forks source link

Sniff optimization #81

Open dingo-d opened 6 years ago

dingo-d commented 6 years ago

We could try to optimize theme sniffing.

Some of the factors for the sniff speed are (by Juliette Reinders Folmer)

  1. Size of the file - very large files take longer to tokenize and obviously longer to run as there are a lot more tokens which may trigger sniffs
  2. Number of sniffs and complexity of the sniffs being run.
  3. Type of files - if you're not including sniffs which examine JS files, make sure PHPCS ignores them instead of tokenizing them and then not having a sniff to examine them. On that note: minified JS will always cause problems.
  4. The system on which things are being run obviously influences things as well. PHP 7 is much faster than PHP 5.
  5. When using PHPCS 3.x, you can try to use the --parallel=xx option to try and speed things up by running checks, well, in parallel

What could be tested to see if the sniffs will take less time

Using classes and namespacing with autoloading could give some minor performance boost.

DannyCooper commented 5 years ago

+1

Could an option be added to use phpcs --extensions=php? It would be useful when the theme is causing Fatal error: Allowed memory size of 134217728 bytes exhausted.

dingo-d commented 5 years ago

We sniff css and js as well, and I have a safeguard to exclude minified files which will prevent these fatal errors. If you check out the version from the PR you shouldn't have these issues 🙂

DannyCooper commented 5 years ago

You're way ahead of me @dingo-d 🤩

DannyCooper commented 5 years ago

On second thoughts, authors do strange things with JS and CSS that can't always be predicted. Causing Allowed memory size errors.

So I think a checkbox for using --extensions=php does have value. Or a field to specify which file types to run the sniffs on.

dingo-d commented 5 years ago

I agree, I'll see if I can implement JS and CSS checking using eslint and stylelint using WordPress standards. That way I should probably avoid memory issues. Thanks!

dingo-d commented 5 years ago

I've given some thought about linting JS and (S)CSS using eslint and stylelint, and I don't think this will be possible, as I cannot know if a user has (in most shared hostings they don't) Node.js, and those linters need it to run. Locally that could work, but if someone wants to use it on a live server that could be tricky.

I'll need to strengthen the minified files check somehow.

joyously commented 5 years ago

For the plugin, couldn't it use the same thing that WordPress is using in the code editors and Additional CSS? It's Code Mirror, bundled, isn't it? Not sure if that might help.

dingo-d commented 5 years ago

You mean the css in the customizer? I'd have to look into that, never poked around that part of the core 😄

timelsass commented 5 years ago

yeah the core editor uses codemirror, and I think the parse is custom using espirma as the base to provide very basic linting, so it wouldn't be near eslint or WordPress standards (not sure if this has changed since 5.0). It used to use JSHint, but I think there were some licensing issues with that. Another thing would be es6, jsx considerations as well. Currently requirements for themes are to include source files - so those would typically cause additional errors if the parser isn't accounting for them and strictly using es5 standards.