Make languageNamed accept display names and extensions again#2883
Open
schani wants to merge 1 commit into
Open
Make languageNamed accept display names and extensions again#2883schani wants to merge 1 commit into
schani wants to merge 1 commit into
Conversation
The v23.1.0 rewrite of languageNamed()/isLanguageName() (da1238c) matched only the lowercase entries in each language's names array and threw "Unknown language name" on a miss. Before that, languageNamed() matched names, displayName, and extension, and returned undefined for unknown names. That broke consumers passing display names — most visibly app.quicktype.io, which stores the chosen language's display name (e.g. "TypeScript") in localStorage and crashed to a blank page for returning users. Restore the old behavior: match names, display names, and extensions case-insensitively (names take precedence over extensions so "js" still resolves to JavaScript, not Flow), and return undefined instead of throwing. An overload keeps the precise LanguageNameMap return type for calls with a known LanguageName literal while allowing arbitrary strings to return TargetLanguage | undefined. Add a regression check to the test harness, since fixtures only ever pass exact lowercase language names and cannot catch this. Fixes #2769 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
86d5347 to
0055534
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
languageNamed()andisLanguageName()in quicktype-core stopped accepting display names ("TypeScript", "C++", "JSON Schema") and file extensions ("kt"), and started throwingUnknown language name: ...instead of returningundefinedfor unknown names.Most visible fallout: app.quicktype.io stores the chosen language's display name in localStorage and calls
languageNamed()with it on load, so returning users got a blank page until they cleared localStorage.Root cause
The v23.1.0 rewrite (da1238c) of
packages/quicktype-core/src/language/All.tschangedlanguageNamed()to match only the lowercase entries of each language'snamesarray and to throw on a miss. Before that, it matchednames,displayName, andextension, and returnedundefinedwhen nothing matched.Fix
Restore the old matching behavior in
All.ts:names, thendisplayName, thenextension, all case-insensitively;"js"still resolves to JavaScript rather than Flow (whose extension is"js");undefinedfor unknown names instead of throwing (all in-repo callers already guard withisLanguageName()ordefined(), andgetTargetLanguage()inRun.tsstill reportsDriverUnknownOutputLanguagefor unknown names);LanguageNameMap[Name]return type via an overload for calls with a knownLanguageNameliteral, while plain strings getTargetLanguage | undefined;isLanguageName()now delegates tolanguageNamed(), so it accepts the same spellings.Tests
The regression test was written first and demonstrated failing before the fix, on top of current master:
test/check-language-named.ts(wired intotest/test.tsalongside the existingcheckJavaEnumAcronymCasingguard) asserts thatlanguageNamed()/isLanguageName()resolve lowercase names, display names, and extensions case-insensitively, and returnundefined/false— not throw — for unknown names. The fixture harness only ever passes exact lowercase names, so no fixture input can catch this; asserting on the API directly is the right slot.ts-node --transpile-onlyit reports 16 runtime failures, e.g.languageNamed("TypeScript"): expected TypeScript, got throw: Error: Unknown language name: TypeScript.--transpile-only).Verification
require("quicktype-core").languageNamed("TypeScript")threw before, now returns the TypeScript language;languageNamed("nope")returnsundefined.node dist/index.js --lang TypeScript sample.jsonnow works;--lang nonexistentstill errors withUnknown output language nonexistent.FIXTURE=typescript-zod script/testpasses (210 tests), which also runs the new check.packages/quicktype-vscodetype-checks cleanly against the new signatures (tsc --noEmit).Fixes #2769
🤖 Generated with Claude Code