statelyai / xstate-tools

Public monorepo for XState tooling
183 stars 40 forks source link

Typegen generates excessive code with tsc #91

Closed mdpratt closed 2 years ago

mdpratt commented 2 years ago

The current implementation of TypeGen works by appending as import("./machine.typegen").Typegen0 to machines schemas. As this is a regular import and not an import type request, most bundlers will automatically generate JS files for the typegen.js. This happens even if you explicitly exclude them in the tsconfig's exclude property, as imports supersede an exclusion.

This means that you could potentially have many empty files appended to your bundles.

// Sample code
"use strict";
// This file was automatically generated. Edits will be overwritten
Object.defineProperty(exports, "__esModule", { value: true });
//# sourceMappingURL=machine.typegen.js.map

The recommended approach is to put add import type { typegen } from './machine.typegen'; instead

Andarist commented 2 years ago

Could you create a full runnable repro case for this problem? I would like to understand the very exact configuration/setup that you are using there because you mention bundlers but it looks like perhaps this is an issue with the TS compiler.

This might also be potentially an issue with TS emit and not necessarily with the chosen approach here. Have you tried changing the value of the importsNotUsedAsValues setting? Does it change anything?

mdpratt commented 2 years ago

Good callout regarding tsc. I'll try to put together a repo but it will be a few days. I also confirmed that setting "importsNotUsedAsValues": "remove" does not prevent the files from being generated, since as import isn't smart enough, apparently.

I'll attach my tsconfig:

{
  "$schema": "https://json.schemastore.org/tsconfig",
  "extends": "@tsconfig/node16/tsconfig.json",
  "include": ["./src"],
  "exclude": ["./src/*.typegen.ts"],
  "compilerOptions": {
    "outDir": "lib",
    "declaration": true,
    "declarationMap": true,
    "forceConsistentCasingInFileNames": true,
    "isolatedModules": true,
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "sourceMap": true,
    "importsNotUsedAsValues": "error", // tried with "remove" as well
    "esModuleInterop": true,
    "noFallthroughCasesInSwitch": true,
    "noImplicitAny": true,
    "noImplicitReturns": true,
    "noUnusedParameters": true
  }
}
mdpratt commented 2 years ago

It actually gets worse than that. I'm writing a generator program that takes a base machine and then builds thousands of machines with different configurations. When using tsc it's actually embedding the entire typegen into the definition file. This is taking what could be a ~2.2KiB file and turning it into ~27Kib!

Since I'm generating 11,237 machines, it's taking 4.6Mb of TypeScript and turning it into ~37Mb of code. This can also cause issues for VSCode's TS engine since it's now storing massive amounts of type data for every machine, instead of sharing a common definition.

It's not the end of the world in theory, since the JS output is fine and the d.ts files should be excluded from any production builds, but it's an interesting problem.

Here is an example of a single machine's declaration. Notice the machine and events both import the full typegen:

export declare const cancelJob: (settings: SdkMachineUserSettings, request?: Braket.CancelJobRequest | undefined) => {
    machine: import("xstate").StateMachine<import("@amzn/xstate-aws-sdk-machine").SdkMachineContext<Braket.CancelJobRequest, Braket.CancelJobResponse>, any, {
        type: "PAGE";
    } | {
        type: "SEND_REQUEST";
        request: Braket.CancelJobRequest;
    }, {
        value: any;
        context: import("@amzn/xstate-aws-sdk-machine").SdkMachineContext<Braket.CancelJobRequest, Braket.CancelJobResponse>;
    }, import("xstate").BaseActionObject, {
        sendCommand: {
            data: import("aws-sdk/lib/request").PromiseResult<Braket.CancelJobResponse, import("aws-sdk/lib/error").AWSError>;
        };
    }, import("@amzn/xstate-aws-sdk-machine/lib/machine.typegen").Typegen0 & {
        indexedActions: import("xstate").IndexByType<import("xstate").BaseActionObject>;
        indexedEvents: import("xstate").IndexByType<{
            type: "SEND_REQUEST";
            request: Braket.CancelJobRequest;
        } | {
            type: "PAGE";
        } | ({
            type: "done.invoke.sendCommand";
        } & {
            data: import("aws-sdk/lib/request").PromiseResult<Braket.CancelJobResponse, import("aws-sdk/lib/error").AWSError>;
        })> & Pick<{
            "": {
                type: "";
            };
            "done.invoke.sendCommand": {
                type: "done.invoke.sendCommand";
                data: unknown;
                __tip: "See the XState TS docs to learn how to strongly type this.";
            };
            "error.platform.sendCommand": {
                type: "error.platform.sendCommand";
                data: unknown;
            };
            "xstate.init": {
                type: "xstate.init";
            };
        }, "" | "error.platform.sendCommand" | "xstate.init">;
    }>;
    events: {
        SEND_REQUEST: <TRequest_1>(request: TRequest_1) => {
            type: "SEND_REQUEST";
            request: TRequest_1;
        };
        PAGE: () => {
            type: "PAGE";
        };
    };
};
Andarist commented 2 years ago

Is the output any different if you are using a regular import~ for this typegen though? Since we are talking about .d.ts files I would suspect both to behave in the same way. I think there are some ways to prevent TS from inlining the type - but to experiment further I would really need to get a repro case for this.

mdpratt commented 2 years ago

I think you're right, I was misunderstanding how TS builds it's types. I've prepared a sandbox here: https://codesandbox.io/s/jolly-shtern-sp9bnt?file=/README.md

This might be a unique problem only impacting me because of the number of machines I have - if someone only had a few dozen, it probably wouldn't matter.

Since I'm already generating the clients as .ts, an alternative approach is for is for me to just create templates for the .js and .d.ts files instead. This would mean I could avoid using tsc for transpiling at all.

Andarist commented 2 years ago

Ok, I see - so TS "inlines" our lengthy~ type into your .d.ts cause it has to write a type returned from the createMachine call. Note that it doesn't actually inline the whole typegen stuff, it only inlines some parts of our ResolveTypegenMeta type: https://github.com/statelyai/xstate/blob/69cc91b411e2cd22c0f6afde386da2dbf22f992d/packages/core/src/typegenTypes.ts#L167-L196

I think it should be possible to trick TS here by using interfaces over types but it also might require some adjustments of this "type machinery". I'm not completely sure that it will actually work but, in general, TS should be much more cautious about precomputing interfaces as they are always open for extension through namespace augmentation.

This is something I've wanted to check out anyway because it could make some types more "readable" in IDE tooltips and such. Could you report this to the XState repo itself? I can't promise that I will look into it right away but I will try to do something about it in the following weeks.

PS. your use case & setup here is quite interesting, it deserves a blog post or something 😉

mdpratt commented 2 years ago

Thanks for the insight @Andarist; I can open an issue in the main repo linking back to this one.

PS. your use case & setup here is quite interesting, it deserves a blog post or something 😉

Right now I'm working on the core mechanics of the library and there are a few extra features I'm ironing out. Once that's done, my intent is to get it published as an official AWS npm package that anyone can import into their codebase. 😀 Once there, I'll see about publishing the creation process.

mattpocock commented 2 years ago

@mdpratt OK if I close this? Closing pre-emptively :)

mdpratt commented 2 years ago

Yup, sounds good. I still need to open a ticket in the main repo about cleaning up the types.