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

deno publish: linter detect slow types on non-public API if keyof typeof is used on unexported member #23646

Closed
lowlighter opened this issue May 2, 2024 · 4 comments
Labels
working as designed this is working as intended

Comments

@lowlighter
Copy link
Contributor

Using keyof typeof on an unexported member seems to expose it totally as part of public API even though it shouldn't


In the minimal repro below, qrcode() is supposed to be the only function exported. It has explicit typing so the fact that QrCode.from is implicit shouldn't matter:

export function qrcode(options?: options): string {
  return QrCode.from(options)
}

type options = {
  ecl?: keyof typeof QrCode.ERROR_CORRECTION_LEVEL
}

class QrCode {
  static from(options?: options) {
    return `${new QrCode(options)}`
  }
  private constructor({ ecl = "LOW" } = {} as options) {
    this.ecl = ecl
  }
  readonly ecl
  static readonly ERROR_CORRECTION_LEVEL = { LOW: {} }
}

It triggers the following errors, which shouldn't happen as class QrCode is not exported.

error[missing-explicit-return-type]: missing explicit return type in the public API
  --> /workspaces/libs/test.ts:10:10
   | 
10 |   static from(options?: options) {
   |          ^^^^ this function is missing an explicit return type
   = hint: add an explicit return type to the function

  info: all functions in the public API must have an explicit return type
  docs: https://jsr.io/go/slow-type-missing-explicit-return-type

error[missing-explicit-type]: missing explicit type in the public API
  --> /workspaces/libs/test.ts:16:12
   | 
16 |   readonly ecl
   |            ^^^ this symbol is missing an explicit type
   = hint: add an explicit type annotation to the symbol

  info: all symbols in the public API must have an explicit type
  docs: https://jsr.io/go/slow-type-missing-explicit-type

Removing keyof typeof fixes the issue:

type options = {
-  keyof typeof QrCode.ERROR_CORRECTION_LEVEL
+  ecl?: "LOW"
}

Note that it is the only change performed, the unexported member has not been edited and still has its implicit typings so the issue can be narrowed down to keyof typeof.
Probably the linter mistakenly add it to exposed member when accessed with keyof typeof operators

@dsherret
Copy link
Member

dsherret commented May 2, 2024

Once a symbol is used in the public api (in this case QrCode), then the entire symbol and therefore all its dependencies become part of the public api for type checking and documentation. It would be quiet complex (especially for documentation) and slower to make a more targeted approach like this.

In this scenario, I'd recommend extracting out the ERROR_CORRECTION_LEVEL to a separate declaration that can be shared by the private QrCode class and the public API (which the extracted declaration is now part of)

@dsherret dsherret added the working as designed this is working as intended label May 2, 2024
@lowlighter
Copy link
Contributor Author

But shouldn't the linter check whether the symbol is actually exported ?

Because I'd understand if the keyof typeof itself would be considered a slow type, but in this case it seems to be able to correctly infer the typing so it doesn't make sense that it makes extra effort to check the remaining of an unexported symbol...

I've rewritten my code anyways, but still feels like a bug. It should either work or just forbids the usage of unexported members in "implicitely public" api because it seems kind of unpredictable whether the slow type is going to be triggered or not

@dsherret
Copy link
Member

dsherret commented May 2, 2024

But shouldn't the linter check whether the symbol is actually exported ?

No, it's extremely common for people to write typescript code using symbols that aren't exported, but are part of the public api.

It doesn't identify exported symbols, but symbols that are part of the public API.

because it seems kind of unpredictable whether the slow type is going to be triggered or not

Essentially it examines the exported types and then their dependencies and their dependencies, etc. So any symbol that is used even transitively by an exported type is part of the public API.

@lowlighter
Copy link
Contributor Author

Ok thank a lot for the explanations, make sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
working as designed this is working as intended
Projects
None yet
Development

No branches or pull requests

2 participants