[bug] `const enum` is causing issues in modern TypeScript

NatoBoram commented 3 weeks ago

Please read the FAQ for the bug you encountered.

[X] I have read the existing FAQ

⏯ Playground Link

https://github.com/NatoBoram/bug-report-ast-grep/pull/4/files

💻 Code

import { Lang } from "@ast-grep/napi"

/** Cannot access ambient const enums when `verbatimModuleSyntax` is enabled. */
export const lang: Lang = Lang.TypeScript

/** Type `"TypeScript"` is not assignable to type `Lang`. */
export const lang: Lang = "TypeScript"

🙁 Actual behavior

src/not_assignable.ts:4:14 - error TS2322: Type '"TypeScript"' is not assignable to type 'Lang'.
4 export const lang: Lang = "TypeScript"
               ~~~~

src/verbatim_module_syntax.ts:4:27 - error TS2748: Cannot access ambient const enums when 'verbatimModuleSyntax' is enabled.
4 export const lang: Lang = Lang.TypeScript

🙂 Expected behavior

Building should be successful

Additional information about the issue

It can be worked around by re-declaring a new language map and using this instead of the fake enum.

import type { Lang } from "@ast-grep/napi"

const langs = {
    Bash: "Bash",
    C: "C",
    Cpp: "Cpp",
    CSharp: "CSharp",
    Css: "Css",
    Dart: "Dart",
    Elixir: "Elixir",
    Go: "Go",
    Haskell: "Haskell",
    Html: "Html",
    Java: "Java",
    JavaScript: "JavaScript",
    Json: "Json",
    Kotlin: "Kotlin",
    Lua: "Lua",
    Php: "Php",
    Python: "Python",
    Ruby: "Ruby",
    Rust: "Rust",
    Scala: "Scala",
    Sql: "Sql",
    Swift: "Swift",
    Tsx: "Tsx",
    TypeScript: "TypeScript",
    Yaml: "Yaml",
} as Record<Lang, Lang>

export const lang: Lang = langs.TypeScript

In the auto-generated code, this can be fixed by dropping the const enum and using a type instead.

https://github.com/ast-grep/ast-grep/blob/c28558a2605e131fc1226c70314a9fa8bded3081/crates/napi/index.d.ts#L42-L68

export type Lang =
  | 'Html'
  | 'JavaScript'
  | 'Tsx'
  | 'Css'
  | 'TypeScript'
  | 'Bash'
  | 'C'
  | 'Cpp'
  | 'CSharp'
  | 'Dart'
  | 'Go'
  | 'Elixir'
  | 'Haskell'
  | 'Java'
  | 'Json'
  | 'Kotlin'
  | 'Lua'
  | 'Php'
  | 'Python'
  | 'Ruby'
  | 'Rust'
  | 'Scala'
  | 'Sql'
  | 'Swift'
  | 'Yaml'

I'd want to make a PR, but the file is auto-generated and I don't know Rust.

HerringtonDarkholme commented 3 weeks ago

This file is autogenerated by napi-rs. Would you like to submit a ticket there? Thanks

https://github.com/ast-grep/ast-grep/blob/c28558a2605e131fc1226c70314a9fa8bded3081/crates/napi/index.d.ts#L4

HerringtonDarkholme commented 3 weeks ago

Also this can be fixed by https://github.com/microsoft/TypeScript/issues/52669

NatoBoram commented 3 weeks ago

I tried to dig a bit more and simply adding --no-const-enum seems to be enough. I tested the generated code in my node_modules and it works. We can even do console.log(Lang) to see the full object.

 crates/napi/index.d.ts    | 2 +-
 crates/napi/package.json  | 6 +++---
 crates/napi/tsconfig.json | 1 +
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/crates/napi/index.d.ts b/crates/napi/index.d.ts
index 9749e14c..f1ffc4bb 100644
--- a/crates/napi/index.d.ts
+++ b/crates/napi/index.d.ts
@@ -39,7 +39,7 @@ export interface FindConfig {
    */
   languageGlobs?: Array<string>
 }
-export const enum Lang {
+export enum Lang {
   Html = 'Html',
   JavaScript = 'JavaScript',
   Tsx = 'Tsx',
diff --git a/crates/napi/package.json b/crates/napi/package.json
index 976b0108..4ee2ae40 100644
--- a/crates/napi/package.json
+++ b/crates/napi/package.json
@@ -41,8 +41,8 @@
   },
   "scripts": {
     "artifacts": "napi artifacts",
-    "build": "napi build --platform --release",
-    "build:debug": "napi build --platform",
+    "build": "napi build --no-const-enum --platform --release",
+    "build:debug": "napi build --no-const-enum --platform",
     "prepublishOnly": "napi prepublish -t npm --skip-gh-release",
     "test": "ava",
     "version": "napi version"
@@ -67,4 +67,4 @@
       "TS_NODE_PROJECT": "./tsconfig.json"
     }
   }
-}
\ No newline at end of file
+}
diff --git a/crates/napi/tsconfig.json b/crates/napi/tsconfig.json
index 969d7b3d..0bf07256 100644
--- a/crates/napi/tsconfig.json
+++ b/crates/napi/tsconfig.json
@@ -4,6 +4,7 @@
     "strict": true,
     "moduleResolution": "node",
     "module": "CommonJS",
+    "verbatimModuleSyntax": true,
     "noUnusedLocals": true,
     "noUnusedParameters": true,
     "esModuleInterop": true,

HerringtonDarkholme commented 3 weeks ago

@NatoBoram This looks good! Would you like to submit a pull request?

NatoBoram commented 3 weeks ago

I'd love to!

I have also updated the bug report pull request with a pnpm patch to show that the pipeline passes with this change

https://github.com/NatoBoram/bug-report-ast-grep/pull/4/files#diff-4d00b691ea0a50c727347d567054defe52c441ec1e2cd0b7bb6c835a39b22ce6

ast-grep / ast-grep