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

Expose TypeChecker.getTypeOnlyAliasDeclaration or similar API to detect if a symbol is a "value" or a "type" #57032

Open
6 tasks done
timocov opened this issue Jan 12, 2024 · 20 comments
Assignees
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@timocov
Copy link
Contributor

timocov commented Jan 12, 2024

🔍 Search Terms

"type checker", "getTypeOnlyAliasDeclaration", "detecting value vs type on a symbol"

✅ Viability Checklist

⭐ Suggestion

It would be nice if the compiler would expose TypeChecker.getTypeOnlyAliasDeclaration or similar function that would help to detect if a given symbol is a type or a value (e.g. a class is a type if it was imported/exported via import-type/export-type syntax.

📃 Motivating Example

I'm maintaining https://github.com/timocov/dts-bundle-generator and have the following issue timocov/dts-bundle-generator#290. My problem is that I need to somehow detect if a module export symbol is a type or a value so I can detect whether the type of a symbol changes since its original declaration and its actual export so I can generate type export instead of exporting a value. I asked in the community discord and @jakebailey pointed that the type checker has function getTypeOnlyAliasDeclaration that might help. I've checked with my test cases and it seems it works like a charm. I understand that this function itself might not be what you actually want to expose from the API so I'm happy with either solution.

Also I'm happy to contribute for this feature.

💻 Use Cases

  1. What do you want to use this for? Use it in the tool dts-bundle-generator to address mentioned issue
  2. What shortcomings exist with current approaches? Nothing seems exist
  3. What workarounds are you using in the meantime? I can use the internal (not exposed) API for a while and hope it won't change in the future
@fatcerberus
Copy link

fatcerberus commented Jan 12, 2024

FWIW import type only prevents using the symbol in emitting positions; both type and value meanings are still imported if both exist. So e.g. you can import type a class and still write typeof Class in a type annotation (which pulls on the value meaning but is ok because it doesn’t get emitted).

I don’t know if the above affects the feature request here but I thought it was worth pointing out.

@timocov
Copy link
Contributor Author

timocov commented Jan 12, 2024

FWIW import type only prevents using the symbol in emitting positions

Thanks for bringing this, but this is exactly what I need to differentiate. I'm building a bundler of dts files and the result of dts file should represent exactly what you can find in the js file. If a developer added exporting the type of a class instead of the class itself in their ts file, the dts bundle (the same as generated dts file by the compiler has) should have exporting the type, not exporting the class. At the moment I don't see how it is possible to tell whether a symbol that represents a module export is "type only" or "value".

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Jan 16, 2024
@jakebailey
Copy link
Member

I am not mega familiar with this API; @andrewbranch do you know if this is something that is reasonable to export? It does "do the job", but its API is a little funky for knowing if a particular alias is type only. But, I suppose getting the declaration may be useful in general. It was added back in #45998.

(At some level, simply exposing it would be nice given it's been there since the beginning of type imports and therefore may be useful with older TS versions, kinda like isAssignableTo, but...)

@andrewbranch
Copy link
Member

andrewbranch commented Jan 16, 2024

I am really afraid of exposing that API, but we need to have some solution for declaration bundlers and transformers. @timocov does it work to instead do something like

while (aliasSymbol?.flags! & ts.SymbolFlags.Alias) {
  const typeOnly = aliasSymbol.declarations.find(ts.isTypeOnlyImportOrExportDeclaration);
  if (typeOnly) return typeOnly;
  aliasSymbol = checker.getImmediateAliasedSymbol(aliasSymbol);
}

?

Note: when testing this approach, ensure you have a test with export type * from "./foo", as that’s represented differently internally (there’s no alias symbol for that re-export, so I’m guessing getImmediateAliasedSymbol will skip it and you’ll miss the type-only declaration 😕)

@timocov
Copy link
Contributor Author

timocov commented Jan 16, 2024

I am really afraid of exposing that API

For be frank I don't need this API directly, if it would be something that would return a boolean when a symbol is "type" or "value" it would be more than enough (at least for my use-case). I tried to use symbol.flags & ts.SymbolFlags.Value before but it didn't work as I suppose the compile "calculates" this flag based on its usage (i.e. even if a class is imported via "valued" import if it is used as "type" then it is marked as "type" as the compiler worked before import-type not to generate imports/exports in js files but this is just my assumption).

ensure you have a test with export type * from "./foo", as that’s represented differently internally (there’s no alias symbol for that re-export, so I’m guessing getImmediateAliasedSymbol will skip it and you’ll miss the type-only declaration 😕)

Thanks @andrewbranch, that's good to know. I recently tried to address export * usage problems and I noticed that something is somewhat different with "re-export everything" statement (at least I learned that explicit export overrides any implicit-export-star export with the same name even if export * is after explicit export) 😄

I didn't have such test before, but apparently your approach above isn't working for it (I just added it and there is a diff with getTypeOnlyAliasDeclaration solution). For other test-cases that I added it seems working fine (but it is possible that I'm missing something here as well).

@timocov
Copy link
Contributor Author

timocov commented Jan 16, 2024

I'm ready to use any "workaround" or approach the compiler team suggests, but it feels like that this information cannot be easily extracted from AST (or mostly relying on AST but use symbols as a helper) without handling lots of edge-cases, but also it feels like this information "belongs" to type checker (or a symbol).

If my assumption about symbol.flags & ts.SymbolFlags.Value nature is correct, then probably the API can be something like "whether a symbol is a value or a type, if it is a type, then the reasoning of it (the symbol is for a type, the code explicitly imports/exports it as type, the compiler figured out that it is a type but was targets a possible value)". Not sure if makes any sense for you tho, just trying to brainstorm in case it might help to make a better decision (but please ignore it as your opinion is much more valuable in here) 😄

@jakebailey
Copy link
Member

Do you happen to have a piece of code where getTypeOnlyAliasDeclaration works, such that I can test to see what other options function the same way?

@jakebailey
Copy link
Member

The one thing I'm seeing is that in checkAndReportErrorForUsingTypeAsValue, we don't use getTypeOnlyAliasDeclaration directly, instead we just resolve the symbol (which is public), then just use getSymbolFlags (not public, but calls getTypeOnlyAliasDeclaration) which gives us the "actual" (?) flags where flags & SymbolFlag.Value does actually seem to report if there's a value component that's usable. Of course, I'm moderately confused from the PoV that there's a symbol flags getter when there's also symbol.flags of the same type 🙃

But, it does make me think that maybe some element of that is the export? Not sure...

@timocov
Copy link
Contributor Author

timocov commented Jan 19, 2024

Do you happen to have a piece of code where getTypeOnlyAliasDeclaration works, such that I can test to see what other options function the same way?

@jakebailey that's exactly export type * you've mentioned above:

index.ts:

import { AnotherFoobar } from './re-export-type';
export {
	AnotherFoobar,
};

re-export-type.ts:

export type * from './another-class';

another-class.ts:

export class AnotherFoobar {}

In this example I'm operating with the exports of index.ts module.

I can push a branch with a test case I use and instructions how to run/debug it (it not complicated I promise 😂) if that helps.

then just use getSymbolFlags (not public, but calls getTypeOnlyAliasDeclaration) which gives us the "actual" (?) flags where flags & SymbolFlag.Value does actually seem to report if there's a value component that's usable. Of course, I'm moderately confused from the PoV that there's a symbol flags getter when there's also symbol.flags of the same type

Oh it looks like I followed the correct path initially 😄

@jakebailey
Copy link
Member

A branch would be helpful; mainly I was looking for the test cases and things in your repo that I could plug various different ideas into. But, if it's just the above, I can try just that.

@timocov
Copy link
Contributor Author

timocov commented Jan 19, 2024

It should be just it, but in case if it would be helpful:

  1. Checkout https://github.com/timocov/dts-bundle-generator/tree/debug-ts57032 branch
  2. npm install
  3. npm run test to run all tests

For this particular issue I added 2 test cases export-class-as-type and export-class-as-type-from-import. The issue above can be reproduced in export-class-as-type.

@andrewbranch
Copy link
Member

I think your idea of what a type-only import or export is is still off and it’s leading you a bit astray here. A type-only alias doesn’t change anything about its target symbol. Even the alias symbol itself is not different depending on whether its declaration was type-only or not. To use a type-only import is not the same thing as importing only a type. You can use a type-only import to import a value. Type-only imports and exports are completely unrelated to SymbolFlags. The terminology “is a value” or “is a type” is out of place here; that’s what SymbolFlags tells you but it’s not what you’re interested in.

it feels like that this information cannot be easily extracted from AST (or mostly relying on AST but use symbols as a helper) without handling lots of edge-cases, but also it feels like this information "belongs" to type checker (or a symbol).

It’s kind of the opposite. In fact, looking at the simple example in timocov/dts-bundle-generator#290, a syntactic transform would easily produce the right output; reconstructing it from symbols is much harder. It’s a bit hard to tell what’s the right thing to expose, if anything, without a better understanding of your approach.

@timocov
Copy link
Contributor Author

timocov commented Jan 19, 2024

without a better understanding of your approach.

@andrewbranch Sure, let me elaborate then. The tool traverses all source files from the compilation and for each node detects 1) whether it should be included to the bundle (unrelated to this problem) 2) if so, whether it should be exported and 3) if it should be exported, what names and how it should be done.

As I mentioned, the first detection is not related to this question lets skip it. The second and third ones are related tho. Skipping unrelated details, statements/nodes that can generate anything in the JS file (class, function, variable, enums, namespaces) should be exported only if they are exported explicitly from the entry point. For this purpose what I'm doing is getting entry point exports, and try to find the node symbol in that table - if it is not there then it shouldn't be exported. But if it is there then it is exported and should either have export keyword or re-exported via export {} statement. This is pretty much how it works right now.

Now the issue is that the fact that a node's symbol is in the exports table doesn't necessary mean that in the JS file this node is exported too. It can be exported as a type via any variation of export type/import type syntax.

My idea was that I can do the following:

  • whether a node should be exported or not still can be detected by the exports table lookup
  • but whether it is exported as a type or not can be detected based on whether its "meaning" has changed - it originally it is declared as a "value" (class, function, etc) but its exported symbol represents a "type" (so the transition happened somewhere and it doesn't matter where and how exactly) then it should be exported via any export type syntax
  • if the "meaning" is the same, then we can just export it without export type syntax

Let me know if it is still unclear what I'm trying to do.

@andrewbranch
Copy link
Member

Thanks for that explanation; I think I’m up to speed now.

whether it is exported as a type or not can be detected based on whether its "meaning" has changed - it originally it is declared as a "value" (class, function, etc) but its exported symbol represents a "type"

Yeah, like I said in my last message, this isn’t accurate. The meaning doesn’t change with type-only imports/exports. You really do need getTypeOnlyAliasDeclaration for this, or something like it. But there is an additional subtlety we need to consider.

Suppose we expose an API like aliasSymbolResolvesToTypeOnlyDeclaration(sym: Symbol): boolean. You might think that if that returns true, you should write export type; otherwise just export. But here’s a case where that would fail:

// @filename: types.ts
interface Foo {}
export type { Foo };

// @filename: entrypoint.ts
import { Foo } from "./types";
const Foo = 2;
export { Foo };

When you read the symbol Foo from entrypoint.ts, aliasSymbolResolvesToTypeOnlyDeclaration will return true because it resolves through export type { Foo } in types.ts. But that doesn’t mean that the same symbol doesn’t also have a reified value meaning. Via symbol merging, it has one in entrypoint.ts, so the final bundled export must be written as export { Foo }.

So knowing whether a symbol has a type-only declaration somewhere in its alias resolution chain isn’t enough. Really, the question is this:

Given an alias symbol with a value meaning, is there a type-only declaration between that symbol and the source of the value meaning?

Asking that question of the example above, we can see that the answer should be “no.” The symbol does have a type-only declaration in its resolution chain, but the source of the value meaning never crosses that type-only boundary, so to speak. In other words, resolving only value meanings starting from export { Foo } never hits a type-only declaration; it stops at the local const Foo = 2.

So, something that answers that question directly is probably the API we need to make. (This is a big reason why I don’t want to expose getTypeOnlyAliasDeclaration directly—it’s too easy to use it and assume a positive answer means a symbol can’t be referenced in a value position, and quite hard to explain why that’s not true.)

@timocov
Copy link
Contributor Author

timocov commented Jan 20, 2024

Oh dear, I completely forgot about declaration/symbol merging... Thanks for reminding about this wild beast 😂

Given an alias symbol with a value meaning

To clarify, when you say "symbol with a value meaning" you mean symbol.flags & ts.SymbolFlags.Value thing or "a symbol that represents a value" (i.e. one of its declarations is a class, function, namespace, variable, enum)? I assume its the latter, but wanted to check to be on the same page. If my assumption is correct, then I'm 100% with you on everything you said above.

So, something that answers that question directly is probably the API we need to make. (This is a big reason why I don’t want to expose getTypeOnlyAliasDeclaration directly—it’s too easy to use it and assume a positive answer means a symbol can’t be referenced in a value position, and quite hard to explain why that’s not true.)

Maybe its still a bit off (pls forgive me, you know the context much better than me), what do you think about having this information connected to a module export instead? I know you mentioned that type-only imports/exports aren't quite related to this, but I thought that it is possible that this knowledge could be useful for module exports. What I mean is that the type checker right now can provide a list of symbols that are exported from a module (TypeChecker.getExportsOfModule or somewhat similar symbol.exports), but it doesn't say how exactly this particular symbol is exported and it can be used i.e. as a type-only, or type-and-value. If this information would be available, then I assume that theoretically this may be helpful for the compiler as well (e.g. in the error detection whether particular symbol can be used like a value; I doubt it tho) (I know it is not available right now and I don't think you'll ever have plans to implement it, but theoretically the case could be "stricter" imports when you must specify type or "not type" when you import anything. I know its a silly feature, but its looks like somewhat an extension of import-type feature 😂).

@timocov
Copy link
Contributor Author

timocov commented Feb 3, 2024

is there a type-only declaration between that symbol and the source of the value meaning?

@andrewbranch I think I found an example where this logic might not work. From my understanding, based on modules exports logic, any "specific" export (export { foo } from 'foobar';) should have higher priority over any export-star statements (export * from 'foobar';), so you can do something like:

export * from 'foobar'; // it exports Foo
export class Foo {} // overrides export *, no compilation error

Considering this, we should first check "named" exports, and only then any export * from statements. But export-type statements might have different order from regular non-type ones:

export * from 'foobar';
export type { Foo } from 'foobar';

In this case, export * from adds values to the export thus stays in JS code, while export type { foo } from 'foobar'; is omitted despite "it should have a higher priority". So at the JS level only export * from statement exists, so Foo from the module above is re-exported as "a value", while from AST/types perspective it has "type-only" declaration and we can't rely on just this.

I can't get rid of a thought that "exports table" should contain this information. I really don't want to ended up writing my own modules resolution mechanism to detect if an export/import can be used as a value or type, considering how complex exports systems are and that the compiler already knows all of this. Can you help me understand why this isn't a solution we're looking for please? Thanks!

@andrewbranch
Copy link
Member

What you just described is a bug. You should be able to access Foo in emitting positions in your example.

@timocov
Copy link
Contributor Author

timocov commented Feb 5, 2024

You should be able to access Foo in emitting positions in your example.

So "valued" export should always have higher priority over any "type" export with the same name (for the same symbol?)? In my example, even if I have export type { Foo } from 'foobar'; it doesn't change "the nature" of an export to "type only", because there is a higher priority export (one that exists at js level, not just types).

@andrewbranch
Copy link
Member

Yeah. My intention is to have zero false positives for the “you can’t use this import because it’s type-only” error. The only relevant question to showing that error is “will it work at runtime”

@timocov
Copy link
Contributor Author

timocov commented Feb 5, 2024

Agreed. Then it feels like your proposed question "Given an alias symbol with a value meaning, is there a type-only declaration between that symbol and the source of the value meaning?" should be corrected? E.g. in the example above there is a type-only declaration, but it doesn't really affect anything. Unless the compile will remove that "useless type-only export that doesn't affect output".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants