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

Move away from Grunt. #21

Open
the-t-in-rtf opened this issue Oct 6, 2020 · 3 comments
Open

Move away from Grunt. #21

the-t-in-rtf opened this issue Oct 6, 2020 · 3 comments
Labels
enhancement New feature or request

Comments

@the-t-in-rtf
Copy link
Collaborator

As @amb26 and I have frequently discussed, our reliance on Grunt for linting has created a few key pain points. For example, Grunt can only find plugins that are installed as a top-level dependency in the node_modules directory. Depending on how npm chooses to stash the dependencies of fluid-grunt-lint-all, downstream users may have to add a dev dependency on one or more Grunt plugins.

We are also fighting other assumptions baked into Grunt. This package works around the globbing and includes/excludes used by Grunt.

A key thing to explore as an alternative is converting this package to work with npx, i. e. a new fluid-lint-all package that has syntax for running all checks, running individual checks, and perhaps flags to control a few key things like which conifguration file(s) to use, whether to report failures as errors or not.

This would also be a good opportunity to come up with a consistent means of configuring all plugins. Ideally we could use something like fluid-glob to consistently express includes and excludes without having to integrate multiple ways of expressing the same intent. We should also see if moving away from Grunt gives us better options in picking up configuration settings from configuration files like .eslintrc.json, as we've discusssed in #16.

@jobara
Copy link
Member

jobara commented Oct 6, 2020

There are some repos that only have grunt to setup the linting, so moving away from grunt would be helpful for those cases too.

I'm thinking a bit more about lint-all and cases like #20 where there is a need to add support for new types of linting. I'm trying to understand the differences between lint-all if run directly through npm vs using each linter independently.

  • Installation
    • Lint-All: pull down all linters in a single step
    • Individual: pull down each linter independently. Would allow for more modularity in choosing which linters to install and run. Including those not supported by lint-all. However, it would require more care to make sure all necessary linters are included.
  • Configuration
    • Lint-All: single configuration file and could group common configuration that is needed by more than one linter. However, this could break configuration of linters run in an IDE as they pick up the standard configuration files for their setup.
    • Individual: known location for configuring linting, would work with linters in IDEs. Could have overlap/duplication of configuration between linters.
  • Running
    • Lint-All: Can easily run in a single command. Could potentially configure to run individual linters.
    • Individual: Can configure an NPM script to run all of the linters in a single step, or run them individually.

Please update and correct any of those thoughts, or add more.

At the moment I'm thinking that lint-all would give us some convenience/consistency in setup, but at the trade-off of modularity and functioning within an IDE. For me, working with an IDE is a big benefit as it helps to catch errors while writing the code, instead of having to wait for linting to run after. Is there a way we can get the best of both worlds. For example ability to pull down common linters in a single step, but be able to add new ones on a per repo basis without having to update the lint-all code. Configuring through the standard lint configuration files instead of our own custom one, so that things work with linters in an IDE and providing a familiar location for modifying the configuration.

Perhaps another path forward would be to work more directly with eslint and look for or make plugins that support all our linting needs through eslint itself. In that way we only need to do the configuration through eslint and can add the plugins as needed. Also configuration can be shared from a standard config like we do with eslint-config-fluid. See: eslint/eslint#13481 for updated config file coming to eslint.

@greatislander
Copy link
Contributor

Another idea to consider is an npm init package to set up linting. I've done this with a project I worked on as part of the WordPress Accessibility team. Specifically, we would set up an npm package whose name begins with create, e.g. create-fluid-lint. Running npm init fluid-lint would then use the create-fluid-lint package to add scripts, dependencies and configuration files to the project where it was run. You can see how this worked for the wp-theme-auditor package here:

https://github.com/wpaccessibility/create-wp-theme-auditor/

This npm capability is documented here: https://docs.npmjs.com/cli/init#description

Additionally, I've found the npm-run-all package helpful for running multiple scripts, much in the way we currently use grunt lint to run multiple lint scripts. For example, if the following scripts exist in package.json:

"scripts": {
    "lint:styles": "stylelint **/*.scss",
    "lint:scripts": "eslint **/*.js",
    "lint:markdown": "markdownlint **/*.md"
}

They can be run in series with the following script if npm-run-all is installed:

"scripts": {
    "lint": "run-s lint:*"
}

@the-t-in-rtf
Copy link
Collaborator Author

As discussed in the CSS and Sass linting pull, this ticket seems a good time to at least consider adding stylus linting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants