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 new rfc for yarn tasks #109

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
103 changes: 103 additions & 0 deletions accepted/0000-yarn-tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
- Start Date: 2018-11-21
- RFC PR:
- Yarn Issue: https://github.com/yarnpkg/yarn/issues/5773

# Summary

Yarn CLI command `yarn run` will be able to orchestrate various npm scripts, allowing them to run in parallel by default and matching any glob patterns.

# Motivation

Many build processes involve Grunt, Gulp, or other task runners that have great use cases but also require separate plugins, configuration files, and understanding of the API. `yarn run` would remove the need for installing these often global dpendencies and lower maintenance costs by providing a simpler interface defined in package.json. This would also cut down on the script run times and would be useful with workspaces as mentioned in https://github.com/yarnpkg/yarn/issues/6689.

Feature Requirements:

- Be able to run multiple commands in parallel across platform
- Show and differentiate console output for each command
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is key. The hardest part of scripts ran in parallel is debugging the log later. "Which script generated the error and when?"

- Compatible with existing behavior of `npm run` command

Prior Art:

- [npm-run-all](https://www.npmjs.com/package/npm-run-all)
- [gulp](https://gulpjs.com/)

# Detailed design

## Simple use case

// In `package.json`

```js
{
"scripts": {
"lint": "eslint .",
"test": "jest"
}
"tasks": {
"verify": ["lint", "test"]
}
}
```

`yarn run verify` would run lint and test commands in parallel

## Advanced use case - pattern matching

```js
{
"scripts": {
"lint": "eslint .",
"test:unit": "jest __tests__",
"test:integration": "jest __integration__"
}
"tasks": {
"verify": ["lint", "test**"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be tempted to drop the globbing feature. Manually specifying each script is clearer and I wouldn't expect these definitions to change frequently or be overly complex.

The feature could be added in a later revision if there was demand for it from the community.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree for a first version it would be simpler not to have globbing

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think it's wise to start w/o the glob too. Solves one of the "unresolved questions" below

}
}
```

`yarn run verify` would run lint, test:unit, and test:integration commands in parallel

## Advanced use case - nested tasks

```js
{
"scripts": {
"build": "webpack",
"lint": "eslint .",
"test": "jest",
}
"tasks": {
"verify": ["lint", "test"],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One slight worry is that lint-staged also uses an array to specify git pre-commit tasks but they use it to mean serial semantics.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could make the tasks an object instead of an array to signify that there's no defined order, but that would be a pain to write.

How big of a deal do you think the issue with lint-staged is?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that an object would be a pain. The lint-staged issue is something that can be handled by documentation so I don't see it as a blocking issue. Of course, there's nothing to say that lint-staged couldn't/shouldn't support parallel tasks and they may adopt whatever syntax yarn chooses.

"dev": ["verify", "build"]
}
}
```

`yarn run dev` would run lint, test, and build in parallel

_NOTE_ Nested arrays or serial runs will not be supported in tasks because they can be done via scripts using `&&` operator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this is a horrible (and confusing) suggestion... what if nested arrays was how it all worked? Something like:

{
  "scripts": {
    "clean": "rimraf ./build",
    "build": "webpack",
    "format": "prettier --write .",
    "lint": "eslint .",
    "test": "jest"
  },
  "tasks": {
    "verify": ["format", "lint"], // <-- Runs "format" and "lint" serially
    "pre-commit": [["verify", "test"]] // <-- Runs "verify" and "test" in parallel
    "dev": ["clean", ["build", "test --watch"]] // <-- Runs "clean", then runs "build" and "test --watch" in parallel
  }
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a bit confusing (lol) but it's not a horrible suggestion. I like it because it broadens the scope of tasks to also solve the problem of just wanting to run tasks serially without having to do yarn lint && yarn test. It helps keep the runner (Yarn) out of the scripts more.

The nested array syntax is a bit funky, but we do see it Babel configurations. Not sure if that helps or hurts the argument though.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking down this path further, perhaps tasks and scripts shouldn't co-exist. If you're wanting to use tasks then it should all be defined there. Formatting can also help show a serial path and parallel tracks:

{
  "tasks": {
    "verify": [
        "prettier --write .",
        "eslint ."
    ],
    "pre-commit": [
        ["verify", "jest"]
    ],
    "dev": [
        "rimraf ./build",
        ["webpack", "jest --watch"]
    ]
  }
}


## Additional flags

`--hide-name`: By default, tasks will run by appending the name of the command to each line. Enabling this flag would allow it to be hidden from the output.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

appending or prepending? I'm imagining something like:

[build] Webpack is starting to compile...
[lint] No errors found.

Similar to the output of Jest when you use multiple projects

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prepending! nice catch.


# How We Teach This

This will be an opt-in feature that would be compatible with existing behavior of `yarn/npm run`. However we should add documentation, as well as a simple migration guide.

# Drawbacks

- Increased complexity of yarn
- Feature not yet supported by npm
- Functionality has been partially implemented as a separate package by other open source projects

# Alternatives

`&` operator, which creates a sub-shell and would not allow modifying the output for readability
Third-party libraries, which are additional dependencies and can require complex command line args
Third-party task runners, which have issues mentioned above, mostly due to dependence on ecosystem/maintainers of plugins

# Unresolved questions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can another unresolved question be the name of the key? Is "tasks" the right name? Hopefully there aren't any prominent libraries using "tasks" already for something else.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, open to any suggestions!


- Order of operations for pre/post hooks for installs if they are defined in tasks. Should they happen in parallel or serial?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would assume serial since that's basically the point. You want the pre hook to happen before the script, not at the same time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of a case like "verify": ["**lint"], if there is a prelint and postlint script. Here user intent is a little unclear and also, not all users are aware of the run hooks. I'm leaning towards just removing pre/post from the matched patterns, since they can explicitly state ["**lint", "postlint"] if they really want to run it in parallel.