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

Annotated type must still be exported #205

Open
Gikkman opened this issue Sep 21, 2022 · 3 comments
Open

Annotated type must still be exported #205

Gikkman opened this issue Sep 21, 2022 · 3 comments
Labels
help wanted Extra attention is needed pr requested

Comments

@Gikkman
Copy link

Gikkman commented Sep 21, 2022

If you have a types file like this:

/** @see {isColor} ts-auto-guard:type-guard */
type Color = "red" | "blue"

And then run ts-auto-guard against the file, it will generate no type guards. But if you add export to the type, like this:

/** @see {isColor} ts-auto-guard:type-guard */
export type Color = "red" | "blue"

Then a type guard file is generated.

I don't know if this is intentional or not, but the README seems to imply the first case should be allowed (since we explicitly whitelist it using annotations). The README even makes a distinction when it says "the --export-all will process all exported types", specifically mentioning them being exported in that case.

I've setup my project so I don't need to import my types explicitly. They'll be imported automatically if I've declared them in my types folder, so I would like to not have to explicitly export them all, if possible.

@rhys-vdw
Copy link
Owner

rhys-vdw commented Sep 27, 2022

Thanks for the report @Gikkman!

it will generate no type guards.

The generator should complain if it sees a type that has the annotation but is not exported, however currently it does not do this because it only loops through exported types.

if you add export to the type, like this [...] Then a type guard file is generated.

It is not possible to create a type guard without importing the type, because it needs it for the signature, e.g.

import { Foo } from "./Foo"; // need this

export function isFoo(obj: unknown) obj is Foo {
}

I don't know if this is intentional or not, but the README seems to imply the first case should be allowed (since we explicitly whitelist it using annotations).

I agree that the README file is a bit of a mess, specifically the section you're talking about. It should be improved. The only real instruction there is hidden in a code comment. It deserves a warning box or something more obvious.

I've setup my project so I don't need to import my types explicitly. They'll be imported automatically if I've declared them in my types folder, so I would like to not have to explicitly export them all, if possible.

I have never heard of such a thing, but that sounds like it would break a bunch of tools, not just ts-auto-guard? Surely other static analysis tools would choke on this (wouldn't they report the types as unused?)

That said I'm out of the loop here so if this is a new option in TypeScript drop a link to the docs. Can either support it or make it clear that this option is not currently supported.

Marking as PR requested -- Can be closed when the README is updated to make the export requirement clearer.

@Gikkman
Copy link
Author

Gikkman commented Sep 28, 2022

I don't think this is a new thing, I've used this design for quite some time. It basically piggy-backs on what is documented here.

I got an example repo I made to showcase kinda how I set stuff up myself (and that shows it working). You can find the example here. I don't get any complaints from my VSCode instance, but maybe other IDEs or tools complain.

@rhys-vdw
Copy link
Owner

Ah, beautiful, thank you! I wasn't aware of this behaviour! To quote from your example:

So basically, as long as the file contains one or more type declarations and no exports and/or imports, that type will be available.

This is something that could be detected at generation time. Hopefully ts-morph has an easy way of checking for global types, but otherwise we could check the file itself for absence of import/export statements and then treat all types as exported.

Sounds like this should be supported then!

The readme should still be updated to clarify the export requirement in the existing use case (and should further explain the global case once supported).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed pr requested
Projects
None yet
Development

No branches or pull requests

2 participants