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

🚀 Feature: Set static version to cli when building #1485

Closed
2 tasks done
rubiesonthesky opened this issue Mar 30, 2024 · 3 comments
Closed
2 tasks done

🚀 Feature: Set static version to cli when building #1485

rubiesonthesky opened this issue Mar 30, 2024 · 3 comments
Labels
area: cli Related to the command line (CLI) area: tooling Managing the repository's maintenance 🛠️ status: in discussion Not yet ready for implementation or a pull request

Comments

@rubiesonthesky
Copy link
Collaborator

rubiesonthesky commented Mar 30, 2024

Bug Report Checklist

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

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;
  }
@rubiesonthesky rubiesonthesky added type: feature New enhancement or request 🚀 area: cli Related to the command line (CLI) area: tooling Managing the repository's maintenance 🛠️ and removed type: feature New enhancement or request 🚀 labels Mar 30, 2024
@JoshuaKGoldberg
Copy link
Owner

need for reading one more file

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

@JoshuaKGoldberg JoshuaKGoldberg added the status: in discussion Not yet ready for implementation or a pull request label Mar 30, 2024
@rubiesonthesky
Copy link
Collaborator Author

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
Copy link
Owner

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 JoshuaKGoldberg closed this as not planned Won't fix, can't repro, duplicate, stale Mar 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: cli Related to the command line (CLI) area: tooling Managing the repository's maintenance 🛠️ status: in discussion Not yet ready for implementation or a pull request
Projects
None yet
Development

No branches or pull requests

2 participants