ngx-rocket / generator-ngx-rocket

:rocket: Extensible Angular 14+ enterprise-grade project generator
https://ngx-rocket.github.io/
MIT License
1.53k stars 216 forks source link

proposal to remove types array from tsconfig.app.json #416

Closed captaincaius closed 5 years ago

captaincaius commented 5 years ago

I'm submitting a...

Current behavior

Currently, the IDE will tell you everything is good, but npm start will fail, because the types array is explicitly empty... The IDE will look at tsconfig.json without the types array, but when you're building, tsconfig.app.json's empty types array will prevent any @types to be used ...

My preference is to just remove it, but an alternative would be to at least put it in tsconfig.json so the IDE doesn't lie to us ;).

Expected behavior

my motivation for posting this is for using @agm/core / googlemaps ... there's an SO answer that describes this better than I can ;).

https://stackoverflow.com/questions/36064697/how-to-install-typescript-typings-for-google-maps/42733315#42733315

If yay, I'll submit a PR.

Minimal reproduction of the problem with instructions

npm install --save @agm/core npm install --save-dev @types/googlemaps in a component, try to use google namespace expected: things work actual: error TS2503: Cannot find namespace 'google'.

Environment

- generator version: @next
- node version: 8.12.0  <!-- run `node --version` -->
- npm version: 6.4.1  <!-- run `npm --version` -->
- OS: Linux - Ubuntu 18.04  <!-- Mac, Linux, Windows -->

Others:

bursauxa commented 5 years ago

In the Stack Overflow answer you posted, I feel like adding 'googlemaps' to the types array would have the same effect (e.g. it works). I feel like performance-wise, it is better to explicitly declare what types you want to use. Although I have not benchmarked it so maybe the difference is negligible.

sinedied commented 5 years ago

The problem with removing the types array for app is that it would make available types from dev dependencies that should not be available for the app, for example the jasmine and node types that are here for unit tests.

So the issue would still exists, just reversed, and putting and empty array in the root tsconfig.json would report errors in unit tests, so it's not simple :/

Also it's more subtle than that: having an empty type array does not prevent @types from being loaded, only implicit (=global) ones: for instance, if I use lodash-es and have installed @types/lodash-es, doing an import map from 'lodash-es/map'; will properly load the type.

IMHO it's still better to keep it like this and make global types import explicit, especially as side-effects between global types definition are quite frequent (it's also the default for ng new generated projects).

captaincaius commented 5 years ago

Understood. Okay thanks friends. Closing :)

PS, if this is the wrong place for bringing up topics like this, feel free to let me know.