search

NullVoxPopuli / fix-bad-declaration-output

Fixes incorrect declaration output
2 stars 1 forks source link

SyntaxError: Missing initializer in const declaration #16

Open simonihmig opened 1 week ago

simonihmig commented 1 week ago

For .js files (with allowJs: true) glint is emitting exports like export const foo: Foo, instead of export declare const foo: Foo (which you have test coverage for), that causes jscodeshift to throw that error. See https://github.com/facebook/jscodeshift/issues/539, which you apparently have seen already.

I don't see a way to fix this properly, other than making jscodeshift be able to parse d.ts files, or reverting to Regex replacements instead of AST transforms?

What kinda helps is no not re-throw here, but just log the error and move on. At least when those js files the d.ts output has been generated for don't import gts...

NullVoxPopuli commented 1 week ago

Re throwing there isn't a hard requirement, i don't think. I expect library authors to be able to read their terminal output during a build. If we remove the throw, i think i'd like than message colored yellow or red or something. Thoughts?

simonihmig commented 1 week ago

Yeah, I patch-package'd it to do that and also output the original e.message. Can add color and file a PR if you want!

NullVoxPopuli commented 1 week ago

that would be lovely! thank you!

I just noticed I never declared node support on this tool. So... we could declare node22+ and then use the builtin styleText? (would be major bump, obvs)

simonihmig commented 1 week ago

TIL about styleText!

Seems it was added in 20.12 tho: https://nodejs.org/api/util.html#utilstyletextformat-text-options Should I end up using this pkg in some non-personal projects, it would be nice to be able to use that version of node! :wink:

NullVoxPopuli commented 1 week ago

node 20 it is! (i have no reason not support it, since it has the features i want haha)