| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
All these you can either add assert that type is not "listOrElement"
https://github.com/microsoft/TypeScript/blob/main/src/compiler/builder.ts#L997
https://github.com/microsoft/TypeScript/blob/main/src/compiler/commandLineParser.ts#L2491
https://github.com/microsoft/TypeScript/blob/main/src/compiler/commandLineParser.ts#L2664
https://github.com/microsoft/TypeScript/blob/main/src/executeCommandLine/executeCommandLine.ts#L166
https://github.com/microsoft/TypeScript/blob/main/src/executeCommandLine/executeCommandLine.ts#L282
https://github.com/microsoft/TypeScript/blob/main/src/services/getEditsForFileRename.ts#L76
Sorry, something went wrong.
There was a problem hiding this comment.
I will look into other comments later
Sorry, something went wrong.
| possibleValues = getPossibleValues(option.element); | ||
| break; | ||
| case "listOrElement": | ||
| if (option.element.type === "string"){ |
There was a problem hiding this comment.
| if (option.element.type === "string"){ | |
| if (option.element.type === "string") { |
Sorry, something went wrong.
| options[opt.name] = validateJsonOptionValue(opt, args[i] || "", errors); | ||
| i++; | ||
| } | ||
| else{ |
There was a problem hiding this comment.
| else{ | |
| else { | |
Sorry, something went wrong.
| resolutionStack = resolutionStack.concat([resolvedPath]); | ||
| const extendedConfig = getExtendedConfig(sourceFile, ownConfig.extendedConfigPath, host, resolutionStack, errors, extendedConfigCache); | ||
| const result: ExtendsResult = { options:{} }; | ||
| if(isString(ownConfig.extendedConfigPath)){ |
There was a problem hiding this comment.
Spaces throughout
Sorry, something went wrong.
| i++; | ||
| } | ||
| } | ||
| Debug.fail(); |
There was a problem hiding this comment.
| Debug.fail(); | |
| Debug.fail("listOrElement not supported here"); | |
Sorry, something went wrong.
| const value = options[name] as CompilerOptionsValue; | ||
| const optionDefinition = optionsNameMap.get(name.toLowerCase()); | ||
| if (optionDefinition) { | ||
| Debug.assert(optionDefinition.type !== "listOrElement"); |
There was a problem hiding this comment.
Please fix formatting for some of the changes that has been indented by one extra tab
Sorry, something went wrong.
There was a problem hiding this comment.
It would be good to add a compiler test that shows the new feature working and doing something useful. It looks like the one changed baseline shows that what used to be an invalid config is now doing something, but it’s hard to see that the multiple extended tsconfigs are actually contributing anything.
Sorry, something went wrong.
|
Example: // @Filename: /tsconfig1.json
{
"compilerOptions": {
"strictNullChecks": true
}
}
// @Filename: /tsconfig2.json
{
"compilerOptions": {
"noImplicitAny": true
}
}
// @Filename: /tsconfig.json
{
"extends": ["./tsconfig1.json", "./tsconfig2.json"]
"files": ["./index.ts"]
}
// @Filename: /index.ts
function f(x) {} // noImplicitAny error
let y: string;
y.toLowerCase(); // strictNullChecks error |
Sorry, something went wrong.
|
Andrew Branch (@andrewbranch) the compiler tests are hard to add with tsconfig. https://github.com/microsoft/TypeScript/pull/50403/files#diff-0556827a3985d4ffd3ed2e51ac63c9d86ba98ab41dca5c7711fb2750188cbe8a shows the test case where multiple tsconfigs are working together ? https://github.com/microsoft/TypeScript/pull/50403/files#diff-97bc6a439e0f02498deab0f7c919e4e7a2f06c6b4832fd2fc7b042f56b141ff9R111 baseline. While said that we could add test in non watch mode as well. |
Sorry, something went wrong.
|
I saw that baseline, but I couldn’t find any indication in the baseline that the options in the extended configs were relevant. I was looking for a strictNullChecks or noImplicitAny error but didn’t see any. Maybe I missed it in the noise. There are plenty of compiler tests with tsconfig files, but I don’t know if there are subtle gotchas with them. |
Sorry, something went wrong.
|
navya9singh you probably want to add test like https://github.com/microsoft/TypeScript/blob/main/src/testRunner/unittests/tsc/forceConsistentCasingInFileNames.ts with the contents of test files as shown by Andrew Branch (@andrewbranch) for better clarity |
Sorry, something went wrong.
|
TypeScript Bot (@typescript-bot) pack this |
Sorry, something went wrong.
|
Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the tarball bundle task on this PR at bb0ae2f. You can monitor the build here. |
Sorry, something went wrong.
|
Hey Daniel Rosenwasser (@DanielRosenwasser), I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so: {
"devDependencies": {
"typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/139933/artifacts?artifactName=tgz&fileId=43D4A075BE928283C15303888F67D4786C1B8B67071DA37D39B422AFBEEC049702&fileName=/typescript-5.0.0-insiders.20221207.tgz"
}
}
and then running npm install. There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.0.0-pr-50403-6".; |
Sorry, something went wrong.
|
It looks like some changes landed in main that require you to update something in your unit tests. I’m guessing it’s #51798. You’ll want to manually merge main into this branch and then take a look at those test files that no longer build, using Sheetal’s PR as a reference for how to update them. |
Sorry, something went wrong.
There was a problem hiding this comment.
Once the build is green, looks good to merge.
Sorry, something went wrong.
|
|
Sorry, something went wrong.
|
Very cool |
Sorry, something went wrong.
|
Just a heads up if you're sharing a tsconfig from a package. It seems typescript@4.9.5 was not restricted by the exports from the package.json file. It is now with @typescript@5.0.3. The tsconfig.json file not found error was the last error when running tsc. In my case, it was also causing eslint (with @typescript-eslint/parser) to fail with out of memory errors. Since we run eslint before tsc, it was a few tries before I decided to skip eslint and run tsc and was able to find the problem. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Fixes #29118