jaredpalmer / tsdx

Zero-config CLI for TypeScript package development
https://tsdx.io
MIT License
11.26k stars 508 forks source link

(fix): remove faulty tsconfig.json include of test dir #646

Closed agilgur5 closed 4 years ago

agilgur5 commented 4 years ago

(test): ensure test/.test.ts and test/.ts files are correctly excluded and don't have declarations generated

(test): ensure types/*.d.ts don't error with rootDir and don't have declarations re-generated either

(env/test): Jest shouldn't run / should ignore tests inside of fixtures


Fixes #638 , the root cause of it, and adds tests for this.

Effectively reverts #226 . I haven't been able to reproduce #225 / #84 in my VS Code, but maybe that's because I have it configured a little differently? Would like to add VS Code integration tests so there's an automated way to check that it works independently on CI without any configuration (and so I don't have to attempt and fail to test this on my own machine). But tsc --noEmit at least, indeed doesn't output errors in test/ anymore. tsdx lint still does. Per my comments in #638 if this and the VS Code integration are problematic, we should move this to a tsconfig.build.json and then have a plain tsconfig.json that extends from it and changes include to './' so everything is type-checked.

kylemh commented 4 years ago

Does this mean we'd also wanna avoid compiling stories (Storybook)?

agilgur5 commented 4 years ago

@kylemh yes, your stories shouldn't be in your include, otherwise they'll be unnecessarily compiled (and output to dist/likely unnecessarily published).

The Storybook template doesn't have them in the include as you can see here. Are yours by chance co-located? We could add **/*.stories.tsx to the default exclude if it's a common enough pattern (I've seen it a few times).

The approach I listed in https://github.com/jaredpalmer/tsdx/issues/638#issuecomment-605728767, especially with file: ['src/index.ts'] is optimal, but unfortunately seems to have issues in upstream rpts2 as a I mentioned there.

kylemh commented 4 years ago

So I’m doing monorepo shit in a personal project first before I try to come up with a template to resolve #122

https://github.com/innocuous-tech/core

I’ve got it all flying for now, but I can’t see how to prevent the stories’ typings from getting compiled into the bundle.

I have it excluded, but it’s not working... what do you think?

agilgur5 commented 4 years ago

Ah yours are like semi-colocated, in the src dir but as separate dirs. I'd leave them outside of src for a template fwiw. Probably just need to mess with the globs a bit. If you moved the exclude into each package it would probably be easier to figure out. Path resolution is a bit weird with extends too but you seem to be using it correctly so I'm not sure.

kylemh commented 4 years ago

Hmmmm I remember reading somewhere that @jaredpalmer prefers colocating tests with source code. Having the answer be “don’t do that” doesn’t seem ideal or in line with the opinions of this repo’s author.

I’ll try manually listing the excludes in each package, but that wasn’t working either.

agilgur5 commented 4 years ago

Well none of the existing templates have co-located tests or stories, so it wouldn't make sense to add a monorepo template that is the odd one out with semi-colocated tests. Would be very inconsistent and confusing to users

kylemh commented 4 years ago

So this is my individual use case. I can make the template match the rest. The point I’m making is that we should be able to have collocated tests and stories.

In my example, test files are ignored but not stories, so I was just tryna make sure it wasn’t something under the hood. Not extending and excluding per package seems to be working. I’ll have to figure out where I’m going wrong with extends

agilgur5 commented 4 years ago

The include/exclude handling should work the same as tsc, the only difference is that TSDX has a default exclude for fully co-located tests (if you're adding your own it doesn't matter). Though it's possible there are bugs with rpts2 as that's one reason why I haven't moved templates to just a files set-up which wouldn't be impacted by location at all, only by imports.