As part of the Bazel toolchain migration we noticed that implicit types
generated by the TypeScript compiler sometimes end up referencing types
from other packages (i.e. cross-package imports).
These imports currently work just because the Bazel `ts_library` and
`ng_module` rules automatically inserted a `<amd-module
name="@angular/x" />` into `.d.ts` of packages. This helped TS figure
out how to import a given file. Notably this is custom logic that is not
occuring in vanilla TS or Angular compilations—so we will drop this
magic as part of the toolchain cleanup!
To improve code quality and keep the existing behavior working, we are
doing the following:
- adding a lint rule that reduces the risk of such imports breaking. The
failure scenario without the rule is that API goldens show unexpected
diffs, and types might be duplicated in a different package!
- keeping the `<amd-module` headers, but we manually insert them into
the package entry-points. This should ensure we don't regress
anywhere; while we also improved general safety around this above.
Long-term, isolated declarations or a lint rule from eslint-typescript
can make this even more robust.
PR Close#61312
As outlined in the previous commit which enabled the `esModuleInterop`
TypeScript compiler option, we need to update all namespace imports
for `typescript` to default imports. This is needed to allow for
TypeScript to be imported at runtime from an ES module.
Similar changes are needed for modules like `semver` where the types incorrectly
suggest named exports that will not exist at runtime when imported from ESM.
This commit refactors all imports to match with the lint rule we have
configured in the previous commit. See the previous commit for more
details on why certain imports have been changed.
A special case are the imports to `@babel/core` and `@babel/types`. For
these a special interop is needed as both default imports, or named
imports break the other module format. e.g default imports would work
well for ESM, but it breaks for CJS. For CJS, the named imports would
only work, but in ESM, only the default export exist. We work around
this for now until the devmode is using ESM as well (which would be
consistent with prodmode and gives us more valuable test results). More
details on the interop can be found in the `babel_core.ts` files (two
interops are needed for both localize/or the compiler-cli).
PR Close#43431
Enables the `esModuleInterop` for all TypeScript compilations in the
project. This allows us to emit proper ESM-compatible code. e.g.
consider the following import:
```ts
import * as ts from 'typescript';
```
This import currently will break at runtime in NodeJS because the
`typescript` package is not shipping ESM. It's still a CommonJS module.
ES modules are able to import from `typescript` though, using an import
statement as above, but everything in `module.exports` is being exposed
as the `default` named export. TypeScript at runtime does not have any
other named exports, so for actual ESM compatibility, all of our imports
need to be switched to:
```
import ts from 'typescript';
```
The `esModuleInterop` option allows this to work even though the
`d.ts` file of TS currently suggests that there are _only_ named exports.
The TypeScript language service will now suggest the correct import form as
shown above. It doesn't enforce that unfortunately, but this commit also
adds a lint rule that enforces certain patterns so that we emit imports
that are compatible with both ESM and CJS output (CJS still needed here
since tests run with CJS devmode output still; this is a future project
to switch that over to ESM!)
PR Close#43431
In combination with the TS `noImplicitOverride` compatibility changes,
we also want to follow the best-practice of adding `override` to
members which are implemented as part of abstract classes. This
commit fixes all instances which will be flagged as part of the
custom `no-implicit-override-abstract` TSLint rule.
PR Close#42512
TypeScript introduced a new flag called `noImplicitOverride` as part
of TypeScript v4.3. This flag introduces a new keyword called `override`
that can be applied to members which override declarations from a base
class. This helps with code health as TS will report an error if e.g.
the base class changes the method name but the override would still
have the old method name. Similarly, if the base class removes the method
completely, TS would complain that the memeber with `override` no longer
overrides any method.
A similar concept applies to abstract methods, with the exception that
TypeScript's builtin `noImplicitOverride` option does not flag members
which are implemented as part of an abstract class. We want to enforce
this as a best-practice in the repository as adding `override` to such
implemented members will cause TS to complain if an abstract member is
removed, but still implemented by derived classes.
More details: https://github.com/microsoft/TypeScript/issues/44457.
PR Close#42512
Since we build and publish the individual packages
using Bazel and `build.sh` has been removed, we can
safely remove the `rollup.config.js` files which are no
longer needed because the `ng_package` bazel rule
automatically handles the rollup settings and globals.
PR Close#28646
Now that we have --noImplicitAny we don't need these checks for explicit types in specific locations.
Also re-enable the check to disallow keywords as variable names.
We've had issues such as the one I documented: https://github.com/Microsoft/TypeScript/issues/5187
This tslint check prevents this happening again.
This change also updates to the newest tslint which gets typings from npm.
Closes#4970
