🚀 Feature: Set static version to cli when building

rubiesonthesky commented 6 months ago

Bug Report Checklist

[X] I have pulled the latest main branch of the repository.
[X] I have searched for related issues and found none that matched my issue.

Overview

I'm proposing that version would be set on compile time to runCli. This would remove need for reading one more file when running the tool. The version should not change between runs. :)

This can be done either by custom esbuild plugin or using some existing esbuild plugin. tsup should support them:

One example plugin

esbuild-plugin-environment

Additional Info

Custom plugin could look like this for example:

import { defineConfig } from "tsup";

import { version } from "./package.json";

const versionPlugin = {
    name: "versionPlugin",
    setup(build: any) {
        return Object.assign((build.initialOptions.define ??= {}), {
            "process.env.VERSION": JSON.stringify(version),
        });
    },
};

export default defineConfig({
    bundle: false,
    clean: true,
    dts: true,
    entry: ["src/**/*.ts", "!src/**/*.test.*"],
    esbuildPlugins: [versionPlugin],
    format: "esm",
    outDir: "lib",
    sourcemap: true,
});

And then we refer to the environment variable in runCli.ts:

    if ({}.hasOwnProperty.call(rawOptions, "version")) {
        runtime.output.stdout(process.env.VERSION ?? "development");
        return ResultStatus.Succeeded;
    }

In the output rucCli.js file, this is just

  if ({}.hasOwnProperty.call(rawOptions, "version")) {
    runtime.output.stdout("0.7.3");
    return ResultStatus.Succeeded;
  }

JoshuaKGoldberg commented 6 months ago

need for reading one more file

Why is that need bad? It's only a few ms, no?

rubiesonthesky commented 6 months ago

Why is that need bad? It's only a few ms, no?

Yeah, it's usually only few ms. I don't think this is issue is in category "This is a problem". On the other hand, why we need to read the file for every time we run the command, when it could be to be static? :D

That said, I'm happy with answer "won't fix".

In the grand scheme, I see that TypeStat could be really fast cli tool. And it could be even installed with homebrew or used with dprint as a wasm. If / when that time comes, then it makes more sense to bundle the app and make all static values as static.

However, maybe in the mean time, this is not something that needs to be done. The code that I provided in the OP works and can be applied whenever.

Oh, yeah! I almost forgot why this may feel unnecessary. You are thinking that package.lock is read only when user uses --version. And I was already in the place where I added that version to all outputs :D So yeah. It does not seem worth it, if it's only used when using --version.

I noticed that typestat has two different welcome messages and I started to unify those. And then it made sense to add version information there too :)

JoshuaKGoldberg commented 6 months ago

Cool! I think we're roughly on the same page for the short-term stuff then - no need to change this now. Thanks for the ideation! 🤗

For long-term, I think the #1341 -> #1314 area of work is top of mind for me: splitting into an enhancement package and an initialization package. But:

really fast cli tool. And it could be even installed with homebrew or used with dprint as a wasm

I don't think that's going to happen 😞. It'd be great, but both the enhancement (especially) and the initialization parts of things will need to use TypeScript type info to be useful. So no matter what sub-second-scale improvements we make prior to that, the TypeScript bottleneck is likely to be orders of magnitudes slower. Even if the nice enhancements native speed tools such as Biome, deno lint, and Oxc are working on (https://www.joshuakgoldberg.com/blog/rust-based-javascript-linters-fast-but-no-typed-linting-right-now) come to fruition.

JoshuaKGoldberg / TypeStat