isaacs / tshy

Other
886 stars 18 forks source link

`liveDev: true` - hard-link the .ts files rather than compiling #69

Closed isaacs closed 5 months ago

isaacs commented 5 months ago

This is an alternate approach to the puzzle described in #66 and #67

Discussion: https://x.com/izs/status/1798492035999261155

Rather than build the files with tsc, in tshy dev mode, it would just hard link the files into place in ./dist/{dialect}/{filename}.ts. Instead of exports like this:

{
  "exports": {
    "./file": {
      "require": {"types": "./dist/commonjs/file.d.ts","default": "./dist/commonjs/file.js"},
      "import": {"types": "./dist/esm/file.d.ts", "default": "./dist/esm/file.js" }
    }
  }
}

It would build it like this:

{
  "exports": {
    "./file": {
      "require": "./dist/commonjs/file.ts",
      "import": "./dist/esm/file.ts"
    }
  }
}

The shim package.json files would be set up the same way that they currently are.

Overrides would be hard-linked to their override file. So, eg, if you have ./src/file-cjs.cts and src/file.ts, then we'd hard-link src/file-cjs.cts to ./dist/commonjs/file.ts and src/file.ts to ./dist/esm/file.ts.

There are likely complications hiding in there, but I think it might sort of Just Work, since ts can import ts files just fine.

isaacs commented 5 months ago

cc: @colinhacks

isaacs commented 5 months ago

Then you could assign the scripts something like this:

{
  "scripts": {
    "prepublishOnly": "tshy",
    "prepare": "tshy dev"
  }
}

Not sure off the top of my head if prepare comes before or after prepublishOnly, but the point would be "publish the prod build, but leave it in dev mode build the rest of the time".

You'd still have to run tshy dev when a file is added or removed, but edits would be picked up immediately, which would be a huge quality of life improvement in monorepo envs.

EDIT: nope, prepare comes after prepublishOnly, so it'd have to be a different scripts setup than this. But I'm guessing there's some way to make it work, and it's just a matter of documenting it properly.

The only indication I can see that it's a publish is npm_command=publish in the environment. So, maybe the solution here is to just leave the scripts as { "prepare": "tshy" }, but then have "tshy": { "devHardLink": true } config, which would do the tshy dev behavior except when npm_command=publish is in the env?

Nice thing about that approach is that then we save having to look at the cli arguments. Having just one command that Does The Thing is quite nice.

colinhacks commented 5 months ago

A few downsides:

I mentioned this on Twitter, but I'm increasingly of the opinion that this is a textbook use case for custom export conditions, and it's easy (and quite satisfying) to configure VS Code, Vite, tsc, Node, tsx, etc to incorporate my special condition.

isaacs commented 5 months ago
  • As you mentioned, you'd still have to run tshy dev when a file is added or removed (this is disqualifying according the goals I originally set out to achieve I'm afraid)

Yeah, I agree that's suboptimal, but I'm not sure how we could manage to keep the CommonJS/ESM hybrid export correct without doing something like this, though.

Like, let's say that there was some option or other that let you end up with this as your exports:

{
  "name": "thing",
  "type": "module",
  "exports": {
    "./foo": "./src/foo.ts"
  }
}

There's no need for the types, and since both require and import would resolve to the same file, we can just set it to a string.

The problem here is that TS will see that export as either CommonJS or ESM, but not both.

So, if you have some other program that has this TS:

import { foo } from 'thing/foo'

Then tsc will complain when building for CommonJS, because it sees that file as ESM, so it will need it to be a dynamic import() in that case. When building for ESM, it'll be fine.

So, ok, let's say we remove the "type": "module" from package.json when we do this kind of dev build.

In that case, it means that all tests, build scripts, and such have to be written in CommonJS style, which is just kind of a pain. (If for no other reason than that it means they can't natively import other ESM code, though the native module system does have some other benefits in certain edge cases.)

[edit to add this graph, cut it from another spot and forgot to paste it back] More importantly, though, if you have a monorepo (the main reason why you'd want this feature), and you're building all the things hybrid-style (the main reason you're probably using tshy), then this will be a huge hassle, because when I try to do a normal non-dev build in a workspace that depends on this one, it'll infer the wrong thing about what it's pulling in.

  • Difficulty of explanation: requires understanding of hardlinks, lifecycle scripts, etc.
  • Relies on setting these scripts in the package.json for all workspace packages. There's always a possibility that someone runs tshy dev followed by npm publish...and if they haven't set up the package.json lifecycle scripts properly this will break things.

Actually, now that I'm thinking about it more, I'd just have it be something like "tshy": { "liveDev": true }, and if that flag is set, then it'll build in dev mode unless process.env.npm_command is either publish or pack. So, when you're preparing the package for deployment, it'll do the normal build, and when you're building it any other time, it'll do a live dev build.

In that case, no one really needs to know about hard links and such, and the scripts setup is still just "prepare": "tshy" as recommended in the docs.

a textbook use case for custom export conditions

This is kind of where I'm leaning as well, which might actually have some other benefits, in addition to whatever kind of liveDev hard linking might be done.

Maybe the exports that tshy generates could include a custom source target or something? Would that be useful?

Like:

{
  "exports": {
    "./foo": {
      "import": {
        "types": "dist/esm/foo.d.ts",
        "source": "src/foo.ts",
        "default": "dist/esm/foo.js"
      },
      "require": {
        "types": "./dist/commonjs/foo.d.ts",
        "source": "src/foo.ts",
        "default": "./dist/commonjs/foo.js"
      }
    }
  }
}

I think it's best to put the source in the import or require block, since they may have different sources in the case of dialect overrides, like:

{
  "exports": {
    "./foo": {
      "import": {
        "types": "dist/esm/foo.d.ts",
        "source": "src/foo.ts",
        "default": "dist/esm/foo.js"
      },
      "require": {
        "types": "dist/commonjs/foo.d.ts",
        "source": "src/foo-cjs.cts",
        "default": "dist/commonjs/foo.js"
      }
    }
  }
}

What would you have to put in tsconfig.json in order to make tools recognize such a thing? Or am I misunderstanding your idea here?

isaacs commented 5 months ago

Ok, hard-linking the source files is kind of magical. Yes, it's annoying to have to rebuild when a file is added, but omg. I did a quick spike of it, and it's (a) not that hard, and (b) amazing. Can jump straight to sources across workspaces, edit files back and forth without rebuilding constantly. I'm totally doing this.

I find that I add files pretty infrequently in workspace dependencies anyway. Usually each package is only a small handful of files, so the hazard isn't too great, and being able to edit them without rebuilding is definitely still a big quality of life improvement. And since TSC properly infers the dialect, you can prod-build one of them while the other is in liveDev mode, and it just works.

colinhacks commented 5 months ago

The problem here is that TS will see that export as either CommonJS or ESM, but not both.

Gotcha! This makes total sense now. So pointing to .ts files in the top level of "exports" seems like a no-go. Definitely seems like a custom export conditions is the way to go, and we can all forget about publishConfig 🙃

Ok, hard-linking the source files is kind of magical

Cool cool cool! Very excited to try this.

Actually, now that I'm thinking about it more, I'd just have it be something like "tshy": { "liveDev": true }, and if that flag is set, then it'll build in dev mode unless process.env.npm_command is either publish or pack

Love this 🙌

Maybe the exports that tshy generates could include a custom source target or something? Would that be useful?

Yes, this is exactly what I'm hoping for. The ability to add a custom export condition that points to the raw source.

I'd recommend making this opt-in. I imagine there are people who would object to a non-standard export condition showing up in their package.json. This would also give users control over the condition name. My suggestion for how this could be configured is in https://github.com/isaacs/tshy/issues/66. (I tweaked it a bit based on your comment.)

Can jump straight to sources across workspaces, edit files back and forth without rebuilding constantly. I'm totally doing this.

This is exactly the experience I'm looking for. And this is achievable without hardlinking using custom export conditions (with a properly configured tsconfig.json). You can see it in action in my sandbox repo. It also will continue to work even when new files are added.

gh repo clone colinhacks/live-typescript-monorepo
cd live-typescript-monorepo/custom-conditions
pnpm i
code .

# then observe the live types updating
# between `packages/pkg-a/index.ts` and `packages/pkg-b/index.ts`

I think it's best to put the source in the import or require block, since they may have different sources in the case of dialect overrides

Makes total sense. I've changed the sample package.json in https://github.com/isaacs/tshy/issues/66 to reflect this.

It may be worth noting that the "source" line must come before "default", otherwise it'll never be resolved (as I'm sure you're aware).

What would you have to put in tsconfig.json in order to make tools recognize such a thing? Or am I misunderstanding your idea here?

To get the "live types" experience in VS Code you'd need to set compilerOptions.customConditions:

// tsconfig.json
{
  "compilerOptions": {
    "customConditions": ["source"]
  }
}

If you want to run a script that imports from one of these workspaces, and you want it to pull from the latest .ts source, you need to use --conditions to tell node to look at your custom condition. (This also uses a loader tsx to transpile the .ts file pre-transpilation.)

node --import tsx --conditions source ./script.ts

I do wonder how something like the tsx would interact with a hardlinked .ts file masquerading as a .js file. The transpile step may not get applied which would make it hard to run code built with "liveDev". I'm not sure if this is the case or not.

isaacs commented 5 months ago

Sweet, thanks for sharing all that. I'll take a look at the repo you linked.

I'd recommend making this opt-in. I imagine there are people who would object to a non-standard export condition showing up in their package.json. This would also give users control over the condition name.

I kind of fall the other way on this. As long as nothing else is using it for some other purpose, export conditions are designed to be a no-op if they're not recognized. So there's really no downside I can see to adding it always (except, idk, a few extra bytes in your package manifest? Whatever, you're already biting that bullet by doing hybrid exports in the first place lol.)

Having it be opt-in means yet another thing to configure, and one of tshy's design goals is to be as "works out of the box" as possible. There's already the opt-in of setting up tools to consume it, so it seems fine to just have it always there. (I realize that tshy is already kind of heavy on configs, I just don't want it to be even more so. It mostly does Just Work by default with no configuration.)

Really, the only hazard is choosing a name that might be interpreted in some different way by another tool. I can't imagine anything else that "source" might refer to, other than the source code that was built (or hard-linked) to the "real" export, but we could make the name configurable if it becomes a problem. (If it's a big problem for a significant number of people, maybe it'll have to be named "tshy" or something by default.) I'm tempted to just ship it as "source", always on, flag it as experimental in the docs, and deal with the complaints if it bothers anyone.

colinhacks commented 5 months ago

That thinking makes a ton of sense to me. 👍

I also haven't seen any conflicts with "source" as a condition name. Another option is to get really explicit and call it "typescript". https://twitter.com/colinhacks/status/1798140505135653304 But I'm ambivalent on this.

isaacs commented 5 months ago

Adding the "source" export condition is super simple. More docs than code, actually. I'll get this pushed out tonight.

isaacs commented 5 months ago

Shipped both features in 1.15.0.

I just realized, the "have to rebuild on files added/removed" is somewhat alleviated by using tshy --watch. Maybe when liveDev: true is set, it should only trigger the rebuild in the watch condition if a file is added or removed, since it'll just be replacing the hard links with identical hardlinks.