Skip to content

TSConfig defaults is a dead code path #84

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dcherman opened this issue Apr 19, 2018 · 3 comments
Closed

TSConfig defaults is a dead code path #84

dcherman opened this issue Apr 19, 2018 · 3 comments

Comments

@dcherman
Copy link
Contributor

The default tsconfig here

const defaultTypescriptConfig = {
'compilerOptions': {
'target': 'es5',
'lib': [
'dom',
'es6'
],
'module': 'es2015',
'moduleResolution': 'node',
'types': ['vue-typescript-import-dts', 'jest', 'node'],
'isolatedModules': false,
'experimentalDecorators': true,
'noImplicitAny': true,
'noImplicitThis': true,
'strictNullChecks': true,
'removeComments': true,
'emitDecoratorMetadata': true,
'suppressImplicitAnyIndexErrors': true,
'allowSyntheticDefaultImports': true,
'sourceMap': true,
'allowJs': true
}
}

Will never be hit since loadSync provides an empty set of defaults if no tsconfig.json is found

https://github.com/TypeStrong/tsconfig/blob/5281eafd459a574343bab1bfb8d2ec004cd07355/src/tsconfig.ts#L142-L148

Which means this condition is never falsy in any scenario

if (!config) {
logger.info('no tsconfig.json found, defaulting to default typescript options')
}
const typescriptConfig = config || defaultTypescriptConfig

We should probably swap that out for resolveSync to see if we would have found a tsconfig.json before attempting to load it if we want these defaults to be used.

@dcherman
Copy link
Contributor Author

Alternatively, it looks like they also provide path on the returned value if it's found from loadSync, else it's undefined. Can easily check to see if a path exists instead.

@eddyerburgh
Copy link
Member

Thanks for the bug report. Would you like to make a PR?

@dcherman
Copy link
Contributor Author

@eddyerburgh Yep, already done. Just had to write some tests / refactor the code a bit to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants