Proposal: Allow configuring wireit through .wireitrc.js

[luke-lacroix-healthy]

I originally suggested this here in #888. However, I wanted to break it out completely so that it could be reviewed separately.

Currently, wireit is only configured through the wireit section of the package.json file. This means that the configuration is static.

However, if wireit allowed for .wireitrc.js file, then I think an entire eco-system is possible. Some examples of where this is useful:

1. detect the package's dependencies within the same mono-repo and add their build scripts to the dependencies array; example: { build: {command: 'tsc --build --pretty', dependencies: loadMonorepoDepencies(__dirname,['lint'])}}
2. create re-usable scripts and script factories - import the script from some library and then use it, example: build: buildTypescriptFactory({root: __dirname, additionalDependencies: ['lint']})
3. add/remove dependencies based on build-environment (ie. maybe we choose not to run lint tasks during image creation and only during CI)
4. allow for dynamically defining environment variables

Those are just the use-cases that would be supported by this relatively simple change.

This would also bring wireit into alignment with tools like Jest, ESLint, and many others which, although they allow for configuration through the package.json file, they also support .js files which can do similar dynamic tasks.

[deebloo]

I wonder if this would introduce issues with things like cacheing. the build would have to freshly execute the script each time it runs a task since it could have changed. When everything is static is really quick to check if something in the config itself is different,

[rictic]

Yeah, we'd need a way to express what the .wireitrc.js file depends on, otherwise we'd never know when it needs to be re-run.

[luke-lacroix-healthy]

Maybe I'm thinking of it incorrectly, but... the `.wireitrc.js` file does not depend on anything. You run the file and it exports the configuration - the same configuration wireit currently reads from the package.json file. So, if wireit is currently hashing/caching the package.json file or the wireit section, it would just hash/cache the output of the `.wireitrc.js` file.

…

On Tue, Nov 28, 2023 at 4:35 PM Peter Burns ***@***.***> wrote: Yeah, we'd need a way to express what the .wireitrc.js file depends on, otherwise we'd never know when it needs to be re-run. — Reply to this email directly, view it on GitHub <#983 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AYANPHZL3P4GSIERIFQXC4TYGZKLZAVCNFSM6AAAAAA76AZ2VOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZQG44DMNZUHA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[deebloo]

The big difference is that the build graph is no longer statically analyzable. I am not super familiar with all of the internals yet but I wonder if there are things that count on config being static. You also can't cache based on the file itself since it would require execution (just another potential difference). You may very well be right and it would work fine.

[luke-lacroix-healthy]

Configuration isn't static though. I can always change my package.json file. So, -if- wireit is performing a check on the wireit section of the package.json, then it can do so on the output of the .js file. Just to be perfectly clear: the .js file can only return static values, not functions nor objects with getters. So, the output is just as statically analyzable as the wireit section of the package.json file.

Another way to think about this: I could create a tool, right now, that would generate the wireit section in the package.json file and then run wireit. If that breaks wireit, then I think there is a bigger issue.

Aside from the discussion about potential impact on caching, if there's no other objections, I'd like to see if I can find the time to submit a PR for this.

[deebloo]

Im not saying that breaks wireit, I am saying it is static in that you can hash the file and compare to determine if anything has changed.

[luke-lacroix-healthy]

I'll take a look at the code. I think it should be as easy as just changing where wireit loads the configuration from, ex.
const config = JSON.parse(await readFile('package.json')).wireit) vs. const config = await require('.wireitrc.js'). Hopefully, the hashing is downstream from that, but I guess I will find out.

[luke-lacroix-healthy]

I was able to get some work done on this.

The only issue I've found is that the configuration is stored in an ASTNode tree. As the configuration that is generated by the JS logic is not a JSON document, the line numbers, etc., will be meaningless. @deebloo do you think this will cause issues?

[deebloo]

not totally sure. Im also still not sure allowing config from a JS file is a good idea based since it makes config dynamic. (I obviously don't speak for the maintainers)

I think we had briefly brought it up before but I wonder if the better way to go would be to write a tool on TOP of wireit to generate config rather than wireit needing to be dynamic itself in that way.

[luke-lacroix-healthy]

not totally sure. Im also still not sure allowing config from a JS file is a good idea based since it makes config dynamic. (I obviously don't speak for the maintainers)

I think we had briefly brought it up before but I wonder if the better way to go would be to write a tool on TOP of wireit to generate config rather than wireit needing to be dynamic itself in that way.

Ahh... Sorry @deebloo . I thought you were one of the maintainers.

The problem with a tool on TOP of wireit is the need to run that tool before running any NPM scripts. Even if you composed your scripts so that this tool always ran first, wireit has already loaded the configuration from package.json and I see no where in the code where it watches the package.json file for changes - it loads the file, once, at startup. It might re-evaluate the package.json when running in watch mode but not every script makes sense as a watch script. This makes the developer experience less than optimal. They have to remember to run a "bootstrap" tool before running the NPM commands or I have to add pre:X scripts for every other script so that NPM runs those before it invokes the wireit script.

So, I would have to create the package dynamic-wireit and, instead of my NPM scripts being in the form of "compile": "wireit", they would have to be "compile": "dynamic-wireit". This package's sole purpose would be to invoke .wireitrc.js and write the output to the package.json file, and then run wireit, being sure to pass along all the command line arguments. Keep in mind that changes to .wireitrc.js in watch mode would not be picked up unless the package had the complexity to watch the file, terminate wireit proper, and then re-launch. Seems awfully complicated for something so trivial to implement in wireit itself.

I do not know who the maintainers are for this package (there's no CODE_OWNERS file) maybe @rictic knows?

[deebloo]

Ahh... Sorry @deebloo . I thought you were one of the maintainers.

No worries! I just really like the tool. I think the primary issue is still static vs dynamic. While the js file might return the exact same format as the json, wireit has no idea when it needs to re-run that script, so it would have to re-run it every time you run a command. For example you run a command, the script is run and wireit can now hash the output for a cache key. If something downstream changes (some file that your script depends on) the current hash is no longer valid, but it can't know that without running the script again.

I am very willing to be wrong here and my concerns could be way off base.

"Your script config stays in your package.json, too. Wireit is designed to be the minimal addition to npm needed to get script dependencies and incremental build."

[rictic]

I do not know who the maintainers are for this package (there's no CODE_OWNERS file) maybe @rictic knows?

It's primarily aomarks, and secondarily myself, andrewjakubowicz, augustjk, and justinfagnani.

The only issue I've found is that the configuration is stored in an ASTNode tree. As the configuration that is generated by the JS logic is not a JSON document, the line numbers, etc., will be meaningless. @deebloo do you think this will cause issues?

The ASTNodes are how we give diagnostics on errors and support most of our IDE features. It makes sense that those features would degrade somewhat if your configuration is dynamic, but we should still strive to give the best errors we can, and to make the IDE still do the best coherent thing in response to IDE actions like Go To Definition. It will be a significant change just by itself to update Wireit to well handle all of the cases where we currently assume that we have ASTNodes.

Maybe I'm thinking of it incorrectly, but... the .wireitrc.js file does not depend on anything.

Well, maybe it does and maybe it doesn't. If it's a Node.js script then by default it can do anything. If it reads files on the filesystem, e.g. by importing JS files or reading its own config files, then we need a system to declare that relationship, so that we know what files to watch in --watch mode, and to avoid re-running the .wireitrc.js file too often (as I suspect they will sometimes grow to be rather slow).

With these points in mind, I wonder if instead it would be better to have tooling to generate the wireit config. You could have a test that you run as part of your test suite that asserts that regenerating the config wouldn't make changes, to ensure that it stays up to date. In addition, we could handle some use cases where you'd reach for generation by having more powerful features in the vanilla Wireit config syntax, so that e.g. you could use wildcards in dependency lists.

[deebloo]

vanilla Wireit config syntax, so that e.g. you could use wildcards in dependency lists.

I like that idea a lot and it would keep the config itself static.

{
   "wireit": {
      "build_all": { "dependencies": ["./packages/*:build"] }
    }
}

would this not introduce some of the same issues we have talked about in this thread?
For example if I added a new package in the above example, but already have a cached build. Wireit would have to always spread the wildcard out to all of its actual dependencies to know when to bust the cache,

[luke-lacroix-healthy]

With these points in mind, I wonder if instead it would be better to have tooling to generate the wireit config. You could have a test that you run as part of your test suite that asserts that regenerating the config wouldn't make changes, to ensure that it stays up to date. In addition, we could handle some use cases where you'd reach for generation by having more powerful features in the vanilla Wireit config syntax, so that e.g. you could use wildcards in dependency lists.

Giving it some more thought... Is this what you mean:

Example package.json:

{
  "scripts": {
    "compile": "wireit",
    "update-wireit": "auto-generate-wireit"
  },
  "wireit": {
    "compile": {
      "command": "tsc -p .",
      "depedencies": {
          "check-wireit-config"
      }
    },
    "check-wireit-config": {
      "command": "auto-generate-wireit --failIfModified"
    }
  }
}

So, npm run compile would fail if the .wireit-generator.js output a different value than what was already in the package but also update the package. The user would then have to run npm run compile again.

Is that what you're thinking, @rictic ?