bolt-cep icon indicating copy to clipboard operation
bolt-cep copied to clipboard

Improve `evalES` types

Open timhaywood opened this issue 2 years ago • 3 comments

I've been using a slightly different evalES function in my own repo, proving nice autocompletion and type-safety between the scripts and calling them in the UI.

Current experience

Currently it's typed here as:

type evalES = (script: string, isGlobal: boolean) => Promise<string>;

And used something like:

evalES(`scriptName(booleanArg, '${stringArg}')`);

Proposed

In my own project, I implemented a custom function that works in the same way, but with a slightly different api and some nicer typings.

TypeScript playground example

import * as scripts from "../jsx";

type evalES<T extends keyof typeof scripts> = (
  scriptName: T,
  ...args: Parameters<typeof scripts[T]>
) => Promise<string>;

Usage

evalES("scriptName", booleanArg, stringArg);

This way you end up with type-safety between ExtendScript and the UI. I also prefer passing the arguments in as values rather than building the string everytime, which helps things wrap nicely in the editor.

Issues

This works fine for me, since I'm only targeting one program (aeft), so there's only one set of scripts. Would need to work out the best way to get this working for mutli-program setups, open to ideas! But will try some things out 👍


Let me know what you think!

Will work up a proper pull request with an implementation soon (if you think it's a good idea) :)

timhaywood avatar May 27 '22 05:05 timhaywood

Nice work @timhaywood! I actually have one almost just like this I was getting ready to release, works fine in dev but fails on build due to the different environments since it tries to import the ExtendScript functions into CEP JS and throws errors like $ and app not defined, etc.

I was trying to import as type, but didn't solve the issue. Did you find a fix for that?

justintaylor-dev avatar May 27 '22 14:05 justintaylor-dev

Had a quick rough go at an initial implementation here: #28

Though yours is probably better! Didn't do anything special. Typechecking is failing for me, where it's not picking up the ExtendScript types properly, since they aren't defined in the JS tree. I assume this is the same issue you where having? If so I'm happy to go ahead and see if I can get it working :)

Few implementation details:

  • I split evalES into two functions evalExtendScript and evalFunction, rather than relying on a isGlobal flag. Personally I like this seperation better, making it nice and clear what the intention is when calling them. Also makes the types simpler 😅

    Alternative

    Interested to see what you came up with, but could potentially combine the two with something like:

    export function evalES<ScriptName extends ScriptNames, Global extends boolean>(
      functionName: Global extends true ? string : ScriptName,
      args: Global extends true ? undefined : Parameters<Scripts[ScriptName]>,
      isGlobal: Global = false as any,
    ): Promise<string> {}
    

    To get correct typings depending upon isGlobal. This would need to be improved though! Not sure this will work exactly.

  • I added some basic argument sanitization in for strings, but this needs work. Not sure what the best way to do it is, maybe to JSON.stringify them? But it seems like a better DX to be able to pass in any string.

Keen to hear your thoughts! Let me know if you think it's headed in the right direction.

timhaywood avatar May 28 '22 01:05 timhaywood

TODO: Return type of evalFunction should probably be:

Promise<ReturnType<Scripts[ScriptName]>>

But it would need to address the possibility that a returned object would need JSON.parse? I need to learn more about how scripts actually work 😬

timhaywood avatar May 30 '22 03:05 timhaywood

@justintaylor-dev added a draft PR, #43 . More details in the description and comments in there.

Manually including the ExtendScript types at the root tsconfig fixes almost all the type errors caused by importing types from jsx to js. Since we're not emitting, don't think it will cause an issue?

Remaining errors are caused by conflicting app types between applications, will continue to think if there's a good solution.

timhaywood avatar Nov 29 '22 00:11 timhaywood

resolved in https://github.com/hyperbrew/bolt-cep/pull/53

justintaylor-dev avatar Feb 09 '23 02:02 justintaylor-dev