[typescript] continue to use the default config in development #21966
+85
−49
Conversation
This file contains 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
In elastic#21865 we tried to make a separate config file that would be used for browser TS files by excluding them from the default config file and adding a second that included them. This works fine in the build, but IDE integrations rely on being able to automatically discover a `tsconfig.json` file in a parent directory of the file being edited, which doesn't work when the tsconfig.json file is found, but excludes the file being edited. In this situation IDE integrations silently fallback to the default TSConfig settings, which don't show the types of compiler errors that will become issues in CI, like implicit any warnings. This implements the original strategy we tried in elastic#21865 of mutating the tsconfig.json file before we run the tsc in the build so that the config files select the right files at build time.
spalger
added
review
Team:Operations
jbudz
reviewed
Aug 14, 2018
| @@ -5,7 +5,10 @@ | |||
| "module": "esnext", | |||
| }, | |||
| "include": [ | |||
| "src/**/public/**/*" | |||
| // in the build we populate this to include **/public/**/* but | |||
There was a problem hiding this comment.
is the config loader okay with comments in json? i don't think this will lint
There was a problem hiding this comment.
Yeah, typescript config can have comments, checkout tsconfig.json. It has a bunch of them and always has.
jbudz
reviewed
Aug 14, 2018
| await write(browserProject.tsConfigPath, JSON.stringify({ | ||
| ...browserProject.config, | ||
| include: [ | ||
| ...browserProject.config.exclude, |
There was a problem hiding this comment.
Whoops, copy paste fumble
jbudz
approved these changes
Aug 14, 2018
|
retest |
💚 Build Succeeded |
spalger
pushed a commit
to spalger/kibana
that referenced
this pull request
Aug 14, 2018
…ic#21966) * [typescript] continue to use the default config in development In elastic#21865 we tried to make a separate config file that would be used for browser TS files by excluding them from the default config file and adding a second that included them. This works fine in the build, but IDE integrations rely on being able to automatically discover a `tsconfig.json` file in a parent directory of the file being edited, which doesn't work when the tsconfig.json file is found, but excludes the file being edited. In this situation IDE integrations silently fallback to the default TSConfig settings, which don't show the types of compiler errors that will become issues in CI, like implicit any warnings. This implements the original strategy we tried in elastic#21865 of mutating the tsconfig.json file before we run the tsc in the build so that the config files select the right files at build time. * [tslint] remove browser config from default projects list since it is empty * [build/ts] fix typo
💔 Build Failed |
spalger
pushed a commit
that referenced
this pull request
Aug 15, 2018
… (#21973) * [typescript] continue to use the default config in development In #21865 we tried to make a separate config file that would be used for browser TS files by excluding them from the default config file and adding a second that included them. This works fine in the build, but IDE integrations rely on being able to automatically discover a `tsconfig.json` file in a parent directory of the file being edited, which doesn't work when the tsconfig.json file is found, but excludes the file being edited. In this situation IDE integrations silently fallback to the default TSConfig settings, which don't show the types of compiler errors that will become issues in CI, like implicit any warnings. This implements the original strategy we tried in #21865 of mutating the tsconfig.json file before we run the tsc in the build so that the config files select the right files at build time. * [tslint] remove browser config from default projects list since it is empty * [build/ts] fix typo
|
6.x/6.5: ad3e193 |
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.
In #21865 we tried to make a separate config file that would be used for browser TS files by excluding them from the default config file and adding a second that included them. This works fine in the build, but IDE integrations rely on being able to automatically discover a
tsconfig.jsonfile in a parent directory of the file being edited, which doesn't work when the tsconfig.json file is found, but excludes the file being edited. In this situation IDE integrations silently fallback to the default TSConfig settings, which don't show the types of compiler errors that will become issues in CI, like implicit any warnings.This implements the original strategy we tried in #21865 of mutating the tsconfig.json file before we run the tsc in the build so that the config files select the right files at build time.