RFC: Explore using a centralized script runner abstraction

[wincent]

There is a lot of discussion in #1548 about how to set up packages moving forward, but there is related opportunity that we could explore soon. Creating this separate RFC issue so as not to delay or derail that PR.

Objective

Explore using a centralized script runner abstraction so that we can easily run tasks at root or individual package level, and have painless updates as we evolve our task implementations.

Current set-up

One of the things the generator does is create a package.json from a template with a set of hard-coded build scripts:

"scripts": {
	"build": "babel src --root-mode upward --out-dir lib --extensions .ts,.tsx",
	"build:types": "tsc --project ./tsconfig.declarations.json",
	"prepublish": "npm run build && npm run build:types",
	"start": "webpack-dev-server --mode development",
	"test": "jest --config ../../jest.config.js"
},

This is good because it means we don't have to stop and think about how to set up scripts for every package we create, nor do we risk copy-paste errors that could arise if we weren't using a generator to do the work for us.

However, just say we decide to change how we run the tests or do the build in the future, then every package's "package.json" needs to be updated.

RFC set-up

If we had an abstraction that knew how to do those "script" tasks, then we could use that instead of the individual tools (babel, tsc, npm, webpack-dev-server, jest etc). If the underlying individual tool changed, then we would just update our abstraction in one place and it would take effect in all the packages immediately without any need to edit the "package.json" files. Additionally, with an abstraction in place designed to work in a monorepo, we have a single location that we can imbue with knowledge about the structure of the project, to give it the "intelligence" necessary to work identically at the root level or within individual packages (the latter is something that @bryceosterhaus brought up as being important in that other PR).

That's all very abstract, so what would this "abstraction" look like? I prototyped one way it could look like in a side project:

• The abstraction is just another package called workspace-scripts; it is basically an executable that knows how to run root-level and package-level tasks in the context of a monorepo that uses Yarn workspaces.
• All of the actual work is done by this script; if you look inside it you'll see it's calling eslint, tsc etc.
• In the top-level "package.json", pretty much all the scripts are just calls to workspace-scripts format, workspace-scripts lint etc) (the only reason I haven't made them all into "workspace-scripts" calls is because I haven't done it yet).
  • Running any one of these runs the script globally, for all the packages; eg. yarn lint runs the lints for the whole repo.
  • You can also do yarn workspace-scripts lint PACKAGE1 PACKAGE2... to run the lints scoped to specific packages.
• All packages in the monorepo have "workspace-scripts" as a dev dependency.
  • The "package.json" scripts for each package just callworkspace-scripts: for example, the "test" script here is just workspace-scripts test PACKAGE_NAME.
• Because of the way Yarn workspaces work, the root level doesn't need an explicit dev dependency on "workspace-scripts"; if any subpackage has the dependency, it gets installed at the root too (and this is useful in the case of the sample project because the "workspace-scripts" package itself is developed inside the monorepo; I haven't tried setting up a dependency from the top-level on a nested package, so I don't know how it will behave).
  • This is a bit edge-casey, but it means that "workspace-scripts" itself can't rely on its own built binary to build its binary; so, instead of using a "test" script of workspace-scripts test workspace-scripts, it uses ./bin/index.js test workspace-scripts
  • One final implementation detail that may be specific to this project: at the top-level we also have that chicken-and-egg situation of not being able to use the built build tool to build the build tool 😂, so the "build" task is actually written like this at the top level only: workspace-scripts build || make -j 4 all, so it tries to use the built version if it can but it falls back if necessary.

So that's all just a prototype, but it hopefully shows the idea that you can:

• Use the same tool at root and package levels.
• Scope the operations to run against any single package or set of packages.
• Have scripts definitions that don't need to change in all packages when we change their implementations, because they all have a fixed format of $TOOL $TASK $PACKAGE_NAME.

[matuzalemsteles]

Hey @wincent great ideas... I'm going to close #1107 in favor of this, this was some of the thoughts I had at the time.

[bryceosterhaus]

Great idea to me. This is a very similar idea that we did for portal with liferay-npm-scripts. The pain points are a little different, in portal we wanted to give ease of entry for backend devs and also wanted to avoid having to change every module's npm scripts whenever there was a sweeping change.

For our case here, I think it definitely adds value to the project but I do slightly worry that its an over-abstraction for Clay's specific use case. Main reasons are that the breadth of Clay doesn't seem like it will grow uncontrollably(i.e. portal's modules), granted its easy to say its an over-abstraction now when there is a single package vs a year from now. Also most people working in this repo should have a frontend-bent in skills and having explicit npm scripts can be helpful.

So needless to say, I am cool with going this direction and I am mostly just playing devils advocate in regards to our specific needs.

[wincent]

Seeing as this got mentioned recently, a couple of updates to my "prototype" since opening this issue back in February:

we also have that chicken-and-egg situation of not being able to use the built build tool to build the build tool 😂, so the "build" task is actually written like this at the top level only: workspace-scripts build || make -j 4 all, so it tries to use the built version if it can but it falls back if necessary.

I changed this by splitting it into two tasks, build and bootstrap:

    "bootstrap": "make -j 4 all",
    "build": "workspace-scripts build || yarn bootstrap",

so it will try build-ing first, and if that doesn't work (because the monorepo hasn't been bootstrapped yet), it fall back to bootstrap.

a bit edge-casey, but it means that "workspace-scripts" itself can't rely on its own built binary to build its binary; so, instead of using a "test" script of workspace-scripts test workspace-scripts, it uses ./bin/index.js test workspace-scripts

I don't remember how I fixed this, but now it just works:

    "test": "workspace-scripts test",

So at the time of writing the top-level scripts look like this:

  "scripts": {
    "bootstrap": "make -j 4 all",
    "build": "workspace-scripts build || yarn bootstrap",
    "changelogs:check": "workspace-scripts changelogs:check",
    "clean": "make clean",
    "dependencies:check": "workspace-scripts dependencies:check",
    "dependencies:show": "workspace-scripts dependencies:show",
    "format": "workspace-scripts format",
    "format:check": "workspace-scripts format:check",
    "lint": "workspace-scripts lint",
    "lint:fix": "workspace-scripts lint:fix",
    "prepublish": "workspace-scripts prepublish",
    "publish": "workspace-scripts publish",
    "test": "workspace-scripts test",
    "test:watch": "workspace-scripts test:watch",
    "typecheck": "workspace-scripts typecheck",
    "typecheck:flow": "workspace-scripts typecheck:flow",
    "typecheck:ts": "workspace-scripts typecheck:ts"
  }

and the package-level scripts ((sample)[https://github.com/wincent/js/blob/master/packages/throttle/package.json#L19-L31]) look like this:

  "scripts": {
    "build": "workspace-scripts build throttle",
    "format": "workspace-scripts format throttle",
    "format:check": "workspace-scripts format:check throttle",
    "lint": "workspace-scripts lint throttle",
    "lint:fix": "workspace-scripts lint:fix:check throttle",
    "prepublishOnly": "echo 'Run `yarn publish throttle` from top-level'; false",
    "test": "workspace-scripts test throttle",
    "test:watch": "workspace-scripts test:watch throttle",
    "typecheck": "workspace-scripts typecheck",
    "typecheck:flow": "workspace-scripts typecheck:flow",
    "typecheck:ts": "workspace-scripts typecheck:ts"
  },

[kresimir-coko]

Did we end up making a centralized script runner abstraction as a response to this issue, or did it just stay in the limbo? @wincent The referenced PR only mentions this issue, but wasn't meant to solve it.

[wincent]

Did we end up making a centralized script runner abstraction as a response to this issue, or did it just stay in the limbo? @wincent The referenced PR only mentions this issue, but wasn't meant to solve it.

Nope. I mean, all the links I posted (to another project that takes the suggested approach) are valid, but we never did anything about it in Clay itself.

[kresimir-coko]

...but we never did anything about it in Clay itself.

@wincent should we revive this issue or close it?

[wincent]

@wincent should we revive this issue or close it?

There is also a third option, which is do nothing. You folks who are working on Clay are in the best position to decide which of the three is best.

• If we're literally never going to do this (hard to say), then there is no harm in closing it, and we reduce clutter.
• If we might do it some day, then keeping it around makes sense.
• One thing I probably wouldn't do is "revive" it right now; in general we want to make strategic decisions about which work we prioritize, and not just start working on things because they're there.

[kresimir-coko]

Alright, keeping it open so we know it's something we can work on when the time comes.

[bryceosterhaus]

Yeah I think its best to keep it around but not actively seek it out at the moment.

[julien]

I think the generator-clay-components package is not really necessary, but having a centralized script runner would be great. @matuzalemsteles we should decide if we prioritize this - if not I guess the issue is going to be closed 😢

[matuzalemsteles]

@julien yeah, I also really like the idea of centralizing the build and maybe we could even cut the build time considerably. But I'm thinking now that since we want to centralize components in just one package maybe we don't need that anymore, I think we can close that for now but we can come back to that idea later if we change our minds about something. What do you think?

[julien]

@matuzalemsteles if we manage to get all components in on package, yes we probably don't need it, but I still think we can keep this issue around just to see how far we manage to go.