change: use peerDeps for typescript, tslib

[lbwa]

Shortly, typescript and tslib should always be a member of peerDependencies, rather than dependencies. Otherwise, tsdx user never use their own tslib and typescript due to nodejs module resolution rules.

reproduce

1. install

npx tsdx create tsdx-demo
# choose one template
cd tsdx-demo

2. add some typescript v4 language feature in src/*.ts file, for example, in src/index.ts：

// src/index.ts
type Method = 'GET'
// Lowercase is typescript v4 language feature
export function oops(method: Lowercase<Method>) {
  return method
}

3. build

yarn build
# semantic error TS2304: Cannot find name 'Lowercase'.

But Lowercase actually is typescript Intrinsic String Manipulation Types in v4

4. check all dependencies

// yarn v1.2.x
yarn list --pattern typescript

 yarn list --pattern typescript
 yarn list v1.22.5
 ├─ @typescript-eslint/eslint-plugin@2.34.0
 ├─ @typescript-eslint/experimental-utils@2.34.0
 ├─ @typescript-eslint/parser@2.34.0
 ├─ @typescript-eslint/typescript-estree@2.34.0
 ├─ rollup-plugin-typescript2@0.27.3
 ├─ tsdx@0.14.1
 │  └─ typescript@3.9.9
 └─ typescript@4.2.3

why

Everything go back to normal when I delete typescript dir in <PROJECT_ROOT>/node_modules/tsdx/node_modules/typescript. I wrote some code only supported by typescript v4, but tsdx only require typescript v3x.

tsdx always require tsdx's typescript and tslib, rather than user's own version due to nodejs module resolution rules.

This code always requires typescript from tsdx's dependencies, rather than project's dependencies, that code typescript version never works!!

conclusion

typescript should always be a member of peerDependencies. tslib too.

solution

1. move typescript, tslib into peerDependencies field

2. throw importation error when typescript isn't installed, but tslib only throws a warning.

for tsdx template user

Nothing needs to be done (because tsdx template will install typescript and tslib automatically)

for standalone user (without tsdx template)

1. should always install typescript, tslib manually.

[lbwa]

why not just bump up typescript to v4 #926 ?

The key is tsdx shouldn't help user to make a choice which version user should use. In other words, tsdx shouldn't provide typescript and tslib for better compatibility(eg, practice in next.js).

use resolution field in package.json ? just like #926 (comment)

It's not an elgant solution, and more mental burden. The important thing is that only works with yarn, not npm.

[agilgur5]

@lbwa I appreciate the detailed write-up, but this is a known issue and not as simple as you suggest. For changes as ranging as this, I would definitely recommend making an issue first before jumping into a PR.

Compatibility is a huge issue

Please see #952 (comment) where I've previously answered the question as to why peerDeps aren't used (history, "all-in-one") and as to why peerDeps are not necessarily an optimal solution. In summary, lots of deps and TSDX itself can and do break easily and often due to TS changes, and TS does not follow SemVer and has a breaking change every minor (this would most likely result in more issues, not less). It is not quite like the React example you point out in Next -- React is one of the most, if not the most, stable package out there; even React's breaking changes tend to not break much and be straightforward to upgrade through if necessary.

Compatibility is also the most common type of issue in TSDX by a fairly wide margin, so we generally do not want to make that scenario worse. That is why several of my RFCs and pending changes try to break out TSDX into more components that can be separately updated, better overrided, and even opted-out. But we're not there yet, so this kind of change is particularly early. Even if I wanted to merge this, TSDX itself is not ready or compatible with it.

This change is also very breaking and itself is not compatible with all of the other dependencies TSDX has that rely on TypeScript and rely on a specific version. #926 (comment) shows some of that particular issue -- it does not solely bump TS.
For this change to peerDeps to work, the version range would have to be significantly constrained, which, to an extent, defeats the purpose of the change (it would be in the user's control, but just barely). In the future, a "core" package may be easier to have a wide constraint while individual "presets" or "plugins" or "add-ons" would have smaller constraints that would change more frequently between versions. But there'd still be some legwork to move the community gradually over to that.

Future

As such, at this time I'll be closing this PR as won't fix. Maybe in the future TSDX will be ready for such a change, but it is not at this time.

use resolution field in package.json ? just like #926 (comment)

Yes, that's meant to be a workaround and is listed as such, not a permanent solution.

---

tslib usage

tslib is also practically not used, as we transpile to ESNext only and then the rest with Babel (see #951 (comment) for more details). I can't remember if it's used by other deps, but I also wasn't the one that added it to the templates and it's not entirely clear why it's there (very possible it's a mistake as other issues have suggested so). See also #962 (comment)

[lbwa]

Ok. Thanks for your response and contributions. Your explanation is very helpful.

[thany]

@lbwa

It's not an elgant solution, and more mental burden. The important thing is that only works with yarn, not npm.

See my other comment where I explain how it works fine with NPM.

edit: Just realised you do need a certain recent version of NPM, and maybe that's why it didn't work for you on NPM.