test/ dir is in include, non-spec TS files (e.g. test utils) break build with v0.13 rootDir: ./src

[lookfirst]

Current Behavior

Prior to v0.13 #504 , I could put a file into my test folder that contained utility code for tests and it would get compiled without a problem.

test/TestUtils.tsx

Now, when I tsdx build, I see this error:

rpt2: options error TS6059: File '/mui-rff/test/TestUtils.tsx' is not under 'rootDir' '/mui-rff/src'. 'rootDir' is expected to contain all source files.

Expected behavior

I have a place to put utility code for tests.

Suggested solution(s)

No idea, but you have a list of 3rd parties using this project and it would be nice if you would start to test future versions of tsdx against these projects to see if you break peoples stuff and can then use that to find out how to tell people how to fix their broken stuff.

Your environment

Software	Version(s)
TSDX	v0.13
TypeScript	3.8.3
npm/Yarn	6.13.7 / 1.22.4
Node	v13.11.0
Operating System	OSX Catalina

[agilgur5]

You don't need to (and probably shouldn't) compile it during build if it's just test code (the error says "source files" and this is a test file), you can just import it in your tests and ts-jest will handle it fine.

EDIT: for tl;dr for other readers, remove "test" from your tsconfig.json include similar to what the templates in v0.13.1 have:

tsconfig.json:

{
  "include": ["src", "types"],
   // ...
}

to see if you break peoples stuff and can then use that to find out how to tell people how to fix their broken stuff.

It is listed very clearly as a deprecation in the release and clearly prints a warning. I very much know it can break many people's code, that is the exact reason it is a deprecation with a warning instead of a full breaking change.

The warning says the previous behavior was very buggy (so much so that the more tests we added, the more frequently CI would fail) and tells you how to change. This was a necessary change.

I also do know a good number of the TSDX configs out there, but there are thousands using this, it's impossible to test against all possible uses. I did create #627 recently however.

In the future we could more formally test against some big projects, but keep in mind TSDX is still very young, it is v0.x and will still have many breaking changes ahead. The API surface will evolve and the point of v0.x is to make that clear to consumers that it's still being ironed out and breaking changes will happen more frequently.

Not that it's necessarily a good thing, but plenty of stable v1+ libraries much bigger than TSDX accidentally break things in patches, minors, etc (this occurs in some of TSDX's own dependencies no less like in #511) or make very hard breaking changes that require big migrations. But that'll happen, and every project will fix bugs and learn.

[lookfirst]

You don't need to (and probably shouldn't) compile it during build if it's just test code, you can just import it in your tests and ts-jest will handle it fine.

It doesn't 'handle it fine', hence this issue.

[agilgur5]

It doesn't 'handle it fine', hence this issue

You clearly wrote this issue is about tsdx build with an error message from build. I said you don't need to build test files. You did not mention tsdx test in your issue what-so-ever.

[lookfirst]

tsdx build is failing for me because I have a file in my test directory called test/TestUtils.tsx.

[agilgur5]

Like I said, you don't need to build that file.

I think I see what the issue is though -- the default template include contains the test dir.
That should either be removed (I don't use it like that) or added to the default exclude for builds:

tsdx/src/createRollupConfig.ts

Lines 151 to 162 in e84e8d6

	exclude: [
	// all TS test files, regardless whether co-located or in test/ etc
	'**/*.spec.ts',
	'**/*.test.ts',
	'**/*.spec.tsx',
	'**/*.test.tsx',
	// TS defaults below
	'node_modules',
	'bower_components',
	'jspm_packages',
	paths.appDist,
	],

That also meant test files were being read by rpts2/TS before and excluding them might improve performance of build a bit (.spec and .test were added to excludes in v0.13 as well).

Related to #464 / #472

[lookfirst]

I do notice that the build is faster now. =)

[lookfirst]

If I remove test folder from include and put TestUtils.tsx back into test, then I get other errors around that file. =(

[lookfirst]

Error:(6, 35) TS6059: File '/test/TestUtils.tsx' is not under 'rootDir' '/mui-rff/src'. 'rootDir' is expected to contain all source files.

[agilgur5]

Huh, even when it's not imported by any src/ files?

Not sure if that's because of an upstream bug like ezolenko/rollup-plugin-typescript2#211 or if it's because rpts2's Rollup include passes all *.ts files in.
I'll have to play around with it and see how tsc behaves. Maybe we'll have to exclude test (and __tests__) from build in any case.

You can also override TSDX's default exclude with your own exclude in tsconfig.json. Keep in mind you'll have to add node_modules and dist as the defaults do in the code linked above.

EDIT: when "test" was removed from include, tsdx build stopped giving errors and this was indeed resolved.
The error OP listed above is apparently not from tsdx build but from the IDEA IDE, which OP did not mention until much later...

[lookfirst]

Yea... just try making a non-test file that is only included by test files (and not used by anything in src) work. ;-)

[agilgur5]

ok so I created a test dir with one file in it testUtil.ts and it:

1. Errors once per build format when include has test.
2. Does not error when include only has ['src', 'types'] (annnd turns out the tests all use a tsconfig.json that doesn't include test so I had to add it there)
3. Does not error when include has test, and exclude has **/test/**

So either of the workarounds work. Now the question is whether to change the default exclude or remove test from include...

include also interacts with the linter; without test there, you'll need a separate tsconfig.json for linting, which is suboptimal.
I've also seen the pattern of having test (or __tests__) dir inside of src/, so a default exclude would be helpful for that pattern too.
This would be a performance improvement too, not including files that don't need to be built.

Buuut, it seems that tsc --noEmit, useful for type-checking, will error out regardless if rootDir is ./src, and will do so for any .ts file in test... 😕
Removing test from include means that it won't be type-checked by tsc --noEmit either, which we don't exactly want.
Could do something about both of these with the build exclude as a quick fix...

I also found microsoft/TypeScript#9858, which validates that rootDir is only meant to control the structure of outDir (exactly how it's being used).
In microsoft/TypeScript#9858 (comment), says "Which files are part of your compilation is a separate question that is answered by files and/or include/exclude", so we shouldn't have test since it's not compiled.
But it does need to be type-checked and linted, and it might interact with storybook too...

But it also says in microsoft/TypeScript#9858 (comment) that "[include and rootDir] don't interact: The settings do not affect each other at all." which seems to be quite inaccurate as rootDir throws an error if it's not the include directory or parent of all include directories...

Guh, I'm not sure there's an easy way of reverting #504 without introducing bugs. The only way it'd work is if all package.json main, module, etc were changed to dist/src/ (which is super breaking) or we handled it internally with a tsdx publish command

[agilgur5]

Please ignore the previous comment I made (now deleted), made too many changes at once, so I tested it wrong. rootDir: ['./src', './test'] doesn't work if ./test is also in your include. If it's excluded it will though.

typescript-eslint/typescript-eslint#1350 (comment) also points to using a separate tsconfig.eslint.json (which is what I do) to include files that shouldn't be compiled but should be linted.

Notably this means that for #639 , example wasn't previously linted at all, because it wasn't in your (or the default) include and was only linting TS. Same is true for storybook or in general files outside of include.

Probably want to move to using tsconfig.eslint.json in any case.

[lookfirst]

Thanks for all your work on this. It is painfully clear to me that nobody writes tests. ;-)

[agilgur5]

I understand you're half-joking, but just to respond to that:

1. TSDX is tested. But only build and lint right now.
   We don't have proper code coverage (I'm working on that), but the core is pretty high actually.
   For my part, nearly everything I've written or reviewed has had tests with the PR or in a subsequent PR. I may have written more tests than code, especially after Add Integration Tests #627.
   (fix): set rootDir to './src', not './'. deprecate moveTypes #504 was certainly tested, in fact it made all the tests pass all the time, instead of increasingly failing.

2. TSDX is infinitely configurable though.
   tsconfig.json, eslintrc, babelrc, jest.config.js, tsdx.config.js all configure it and just tsconfig.json itself has an infinite number of combinations. A metric like code coverage doesn't account for that.
   I would like to clamp down on those a bit more with [RFC] Presets #634 and [RFC] Shareable Plugins (for TSDX Build/Config, i.e. tsdx.config.js or Rollup in general) #635, which would also allow for better testing by splitting things up.
   The oversight here is that the test fixtures aren't a 1-to-1 match with the templates, as I realized above.
   (create is also a young feature (with no tests yet) with mostly community contributions. e.g. Add "test" directory to template tsconfigs' includes #226 added the test include, Storybook is entirely community contributions and is very recent).
   The question I've been trying to answer here is which is wrong -- the test or the template (or both). Given (fix): set rootDir to './src', not './'. deprecate moveTypes #504 was necessary and that these issues I've linked to point to some different methods of configuration, it might be that the templates should be changed. Problem is that both configs are valid, but which one is better long-term and more the norm. And that TSDX still needs to be backward-compatible to support old configurations

3. tsconfig.json in particular has a TON of confusion around it as the issues I've linked above show and as have previous issues with it (like paths support).
   The TS community has used features differently than they were originally intended to be used and there has even some confusion from TS maintainers.
   This is partially due to tech debt, with many historical decisions as the JS ecosystem evolved and a need to stay backward-compatible.

4. TSDX is still young.
   See my initial comments -- still v0.x
   There's a lot to work on, much of the things I mentioned here are config and test related, which are the key issues you're pointing to.

[lookfirst]

For the record, I wasn't poking at TSDX, I was poking at users of TSDX. ;-)

[lookfirst]

If your users were writing tests, I'd expect to see a long list of people commenting on this issue right now.

Having a simple helper file that is shared between tests is such a common usecase in every set of tests I've ever written, it seems to reason that people would be commenting like mad right now.

[jaredpalmer]

Thank you both for the back and forth.

[agilgur5]

If your users were writing tests, I'd expect to see a long list of people commenting on this issue right now.

Ah, I see. Well I think there's also less because not everyone has upgraded, and not everyone has test in their include.

So I did some more testing -- having test in the include had other problems already (with rootDir: './'): tsdx build would output type declarations for files in test just like #464 / #472 . They didn't need to be co-located, they just had to be inside the include.
#472 made it so the .spec and .test files didn't have type declarations generated for any include in general (incidentally that was from my review, it was initially only for src/**/* specs), meaning it helped with test include part too.

---

So I think the correct approach to this will be to remove test from include in the templates, i.e. revert #226, as that was buggy already, and issue guidance on that.
The config that makes the most sense actually is files: ['src/index.ts'] as that's the only file that's built. Could include types as well.

Notably tsc --noEmit for type-checking (which might mirror type-checking in the editor integration too; not sure mine is quite configured) and tsdx lint do need to include all TS files so that they're all type-checked and linted.
Linting isn't too difficult, we just create a tsconfig.eslint.json that extends the base but instead include: ['**'] or something, and use it in the default parserOptions. That should be added to --write-file too. (this is basically what I use in my libraries)
Type-checking and editor type-checking are more difficult; while tsc -p can be used to choose a tsconfig.json, the editor might only use the exact tsconfig.json of the directory -- see #84

That's super involved, though notably example and stories weren't linted or type-checked beforehand anyway; practically speaking it only changes how test is handled (and ts-jest already type-checks anything that's imported there as well).
EDIT: example would be type-checked by an editor because it has its own tsconfig.json (differently configured, c.f. #538), but not with tsc --noEmit from root. example TS files weren't linted because the root tsconfig.json didn't include it

A quick-fix that handles this particular case is the suggestion I made before, adding test/ to the default exclude. That's more like a band-aid though, the actual issue is that test/ shouldn't be in include The quick correct solution is to remove it from include, and then we can focus on configuring everything else after, which involves a lot more changes.

[agilgur5]

It is also preventing Formik from using latest release

Also they are just warnings, you can ignore the rootDir warning and continue using ./, just that that usage breaks declaration maps and might occasionally fail to produce all declaration files as we had on CI (you also called that "a BAD bug" 😅 , it's arguably worse since it's silent if you don't have a test for it).

[agilgur5]

Type-checking and editor type-checking are more difficult; while tsc -p can be used to choose a tsconfig.json, the editor might only use the exact tsconfig.json of the directory -- see #84

So maybe it makes sense for root tsconfig.json to have include: ['**'] for linting and type-checking via tsdx lint and tsc --noEmit as well as zero-config editor integration.
And then we have a tsconfig.build.json that's specifically for building, has file: ['src/index.ts'] and maybe include: ['types/'] and would be used by default by tsdx build.
tsconfig.json could extend tsconfig.build.json so that it's the source of truth.

Not sure how Storybook would interact with this however, c.f. #641

[agilgur5]

Confirmed some more cases today that the faulty include of test affects existing libraries:

• mobx-react has test/**/* in their include and published with a test dir in 6.1.8. That's actually not from our templates, which confirms that is a wider spread issue in the TS community in general (and they're certainly more experienced in TS than me, mobx and MST have some crazy types -- immer uses files: [...] instead though).
• mui-rff, which I think is based off our templates (correct me if I'm wrong) and uses test in include, had published a test dir in 2.0.3 and prior, but not in 2.0.4 which uses TSDX v0.13
• The test dir just has type declarations, but those shouldn't be published, and it's wasteful for rpts2 / TS to process the test dir and then compile out declarations. Hence, removing it will also improve performance.

So yea, having test in include confirmed to be problematic a few times now and confirmed to be a wider TS community issue.

It is also preventing Formik from using latest release.

@jaredpalmer I checked out Formik and am not sure what the issue is there, as Formik doesn't have test in include. Doesn't seem like Formik should have any problems with changing rootDir to ./src, but let me know if I'm missing something

---

Currently working on getting #621 and #627 out ASAP as they improve testing and the testing structure.
Then will push out two PRs with tests, one for an initial quick-fix of removing test from the templates' include, and a second for using the tsconfig.build.json with file: ['src/index.ts'] and a tsconfig.json with include: ['**'].
Will need to test at least the latter with Storybook as well.
Planning for the first to land as v0.13.1 and the second ideally v0.13.2 (although it's a big change idk, requires an internal change in tsdx build to support tsconfig.build.json as the default too).
Should also group nicely with planned changes for v0.13.x around noEmit (only for tsconfig.json, not tsconfig.build.json), skipLibCheck, removing redundancies, improving the example tsconfig.json, and adding comments to various tsconfig options to help with understanding why we configured them as such.
Some of those could go out before the tsconfig.build.json change, though that ties into why removing test from include in v0.13.1 is necessary

A preset for tsconfig.json, as part of #634, would also be something helpful to expedite to make future changes to it easier. The custom templates PRs give me some concern about allowing other templates out there that might currently use incorrect configuration too

[lookfirst]

• mui-rff, which I think is based off our templates (correct me if I'm wrong) and uses test in include, had published a test dir in 2.0.3 and prior, but not in 2.0.4 which uses TSDX v0.13

It is based off a template several versions ago.

[agilgur5]

Removed from the template, test added, and added a sentence to the rootDir deprecation warning about this in #646

Per my comments there, I couldn't reproduce the VS Code issues in #225 / #80 and tsdx lint continued to work fine. Though tsc --noEmit indeed stopped reporting errors in test.
Opened an issue around creating integration tests for VS Code in #650 -- not totally sure how to do that myself.

@allcontributors please add @lookfirst for bug

[allcontributors]

@agilgur5

I've put up a pull request to add @lookfirst! 🎉

[lookfirst]

Updated to latest version (0.13.1)

I moved my TestUtils.tsx file from ./src back to ./test.
Removed 'test' from include in tsconfig.json.

Now my IDE reports this error:

"TS6059: File '/mui-rff/test/TestUtils.tsx' is not under 'rootDir' '/mui-rff/src'. 'rootDir' is expected to contain all source files."

[agilgur5]

Yea you gotta remove test from your tsconfig.json include: https://github.com/lookfirst/mui-rff/blob/e8519273740ac48f217b232d8dcd7e1cf606a659/tsconfig.json#L5

v0.13.1 just changed the templates, unfortunately we can't change user tsconfigs yet (but maybe we will once there's a TSDX tsconfig preset)

[lookfirst]

I did remove it.

[lookfirst]

Feel free to clone my repo, fix what you think is broken and make sure to open a test file in IDEA and make sure nothing is broken there too. Then submit a PR. Then you could close this issue. ;-)

[agilgur5]

Ok, might wanna restart your IDE or something, I have periodic caching issues in mine too.

If your include doesn't have test, TS literally doesn't pick it up at all, like tsc --noEmit doesn't even read it.

So either cache issue or there's a bug in the IDE

[lookfirst]

I did clear cache and restart it.

[agilgur5]

I mean I don't know what to tell you, that sounds like an IDE issue -- it's reading your test dir despite that not being included (only other way it'd be included is if you imported it from an included file, which it doesn't seem like you're doing).

If tsc doesn't read it and doesn't error, neither should the IDE.
(You might wanna throw in --skipLibCheck for simplicity, though the @date-io/type error is an error in your dependencies)

[lookfirst]

Let's see what IDEA says about it: https://youtrack.jetbrains.com/issue/IDEA-236281

[agilgur5]

Let's see what IDEA says about it: https://youtrack.jetbrains.com/issue/IDEA-236281

You should probably give them your tsconfig.json as well -- it doesn't include "test" but you're getting an error for things in "test" (and that error itself says "all source files").

And, per previous comments, for some reason only one file is giving that error instead of all "test" files (tsc would give errors for all if the include had "test"), which makes it seem even more like a cache issue.

[agilgur5]

You can also use the approach I mentioned in #638 (comment) , which should make the tsconfig.json work better with all sorts of integrations, but per above, I haven't been able to repro the reason why "test" was in include anyway, so I'm not officially recommending that yet or adding internal changes (especially as it's a relatively big change to user configs).

Summary would be:

• tsconfig.json -> tsconfig.build.json (optionally remove "src" from include and use files: ["src/index.ts"] as a safelist instead... though this apparently has some bugs in upstream rpts2)
• new tsconfig.json:

{
  extends: ['./tsconfig.build.json'],
  include: ['./'], // all files should be checked
}

That'll require passing --tsconfig ./tsconfig.build.json to tsdx build for now. If it's officially adopted, tsconfig.build.json will be read first, with tsconfig.json as back-up if it doesn't exist.