Skip to content

Commit

Permalink
fix(compiler): match tsconfig include paths properly (#4676)
Browse files Browse the repository at this point in the history
this commit fixes a bug where a user was unable to use relative paths in
the `include` property of their `tsconfig.json` file if `srcDir` was
also specified.

previously, when `tsconfig.json#include` contained a relative path:
```
"include": [
  "./app/javascript"
],
```
and `tsconfig.json#srcDir` included the non-relative version of that path:
```
srcDir: 'app/javascript',
```

then you would receive a very confusing error:
```
[ WARN  ]  tsconfig.json "include" required
           In order for TypeScript to improve watch performance, it's recommended the "tsconfig.json" file should have
           the "include" property, with at least the app's "app/javascript" directory listed. For example: "include":
           ["app/javascript"]
```

closes #4667
  • Loading branch information
rwaskiewicz authored Aug 14, 2023
1 parent a62498c commit 664ecb7
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 2 deletions.
35 changes: 35 additions & 0 deletions src/compiler/sys/typescript/tests/typescript-config.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { hasSrcDirectoryInclude } from '../typescript-config';

describe('typescript-config', () => {
describe('hasSrcDirectoryInclude', () => {
it('returns `false` for a non-array argument', () => {
// the intent of this test is to evaluate when a user doesn't provide an array, hence the type assertion
expect(hasSrcDirectoryInclude('src' as unknown as string[], 'src')).toBe(false);
});

it('returns `false` for an empty array', () => {
expect(hasSrcDirectoryInclude([], 'src/')).toBe(false);
});

it('returns `false` when an entry does not exist in the array', () => {
expect(hasSrcDirectoryInclude(['src'], 'source')).toBe(false);
});

it('returns `true` when an entry does exist in the array', () => {
expect(hasSrcDirectoryInclude(['src', 'foo'], 'src')).toBe(true);
});

it('returns `true` for globs', () => {
expect(hasSrcDirectoryInclude(['src/**/*.ts', 'foo/'], 'src/**/*.ts')).toBe(true);
});

it.each([
[['src'], './src'],
[['./src'], 'src'],
[['../src'], '../src'],
[['*'], './*'],
])('returns `true` for relative paths', (includedPaths, srcDir) => {
expect(hasSrcDirectoryInclude(includedPaths, srcDir)).toBe(true);
});
});
});
17 changes: 15 additions & 2 deletions src/compiler/sys/typescript/typescript-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,21 @@ const createDefaultTsConfig = (config: d.Config) =>
2,
);

const hasSrcDirectoryInclude = (includeProp: string[], src: string) =>
Array.isArray(includeProp) && includeProp.includes(src);
/**
* Determines if the included `src` argument belongs in `includeProp`.
*
* This function normalizes the paths found in both arguments, to catch cases where it's called with:
* ```ts
* hasSrcDirectoryInclude(['src'], './src'); // should return `true`
* ```
*
* @param includeProp the paths in `include` that should be tested
* @param src the path to find in `includeProp`
* @returns true if the provided `src` directory is found, `false` otherwise
*/
export const hasSrcDirectoryInclude = (includeProp: string[], src: string): boolean =>
Array.isArray(includeProp) &&
includeProp.some((included) => normalizePath(included, false) === normalizePath(src, false));

const hasStencilConfigInclude = (includeProp: string[]) =>
Array.isArray(includeProp) && includeProp.includes('stencil.config.ts');

0 comments on commit 664ecb7

Please sign in to comment.