microsoft / microsoft/TypeScript
Allowing relative paths in triple slash "types" directives generates broken definitions
@weswigham is already working on this.
Since Feb 26, 2020.
- Dominant language
- Go
- Stars
- 111k
- Forks
- 14.3k
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 132
Description
I managed to publish a module with unusable type definitions due to incorrect use of triple slash directives in one of my dependencies. I think Typescript could and should have caught this as an error when generating declarations during compilation.
TypeScript Version: 3.8.0-dev.20200131
Search Terms:
triple slash directive relative path
Code
Let's build a minimal app that consumes a custom type definition. For convenience I'll place that in types and specify typeRoots but this also applies to published modules which might appear in node-modules/@types/some_module.
├── package.json
├── tsconfig.json
├── src
| └── main.ts
└── types
└── package
├── index.d.ts
└── type.d.ts
package.json
{
"name": "reference-type-paths",
"version": "1.0.0",
"private": true,
"scripts": {
"build": "rm -rf dist/*; tsc"
},
"devDependencies": {
"typescript": "^3.8.0-dev.20200131"
}
}
tsconfig.json
{
"compilerOptions": {
"target": "ES2019",
"module": "commonjs",
"declaration": true,
"outDir": "./dist",
"strict": true,
"typeRoots": ["./types"]
}
}
We setup some TS file to use a custom type defined in a module.
src/main.ts
export class Container {
readonly data: Optional<string>;
}
Unfortunately our module has incorrectly used a types triple slash directive with a path.
types/package/index.d.ts
/// <reference types="./type" />
types/package/type.d.ts
declare type Optional<T> = T | null | undefined;
Finally we run tsc.
Expected behavior:
Typescript should not emit declaration files which contain invalid "types" paths. Use of /// <reference types= with a path (or a path which would not resolve relative to typeRoots?) should be an error during compilation to prevent generating unusable declaration files.
We have /// <reference path= which should have been used here instead. If types/package/index.d.ts contains /// <reference path="./type.d.ts" /> Typescript produces:
dist/main.d.ts
/// <reference types="package" />
export declare class Container {
readonly data: Optional<string>;
}
Which would then resolve correctly.
Alternately a possibly valid but fragile output might be to generate a path relative to the typeRoots directory:
dist/main.d.ts
/// <reference types="package/type" />
export declare class Container {
readonly data: Optional<string>;
}
https://www.typescriptlang.org/docs/handbook/triple-slash-directives.html#-reference-types- states:
The process of resolving these package names is similar to the process of resolving module names in an import statement. An easy way to think of triple-slash-reference-types directives are as an import for declaration packages.
which doesn't suggest that relative paths are inappropriate or dangerous in triple slash type references.
Actual behavior:
Unfortunately what actually get is the preserved relative path from the module's triple slash directive.
dist/main.d.ts
/// <reference types="./type" />
export declare class Container {
readonly data: Optional<string>;
}
This definition is unusable. If we publish it consumers of our module will see errors because type does not appear relative to their typeRoots.
This is especially frustrating because may only be noticed in second level dependents of the module with the incorrect directive. In this example I can depend on package, apparently successfully compile and generate my own declarations, publish them, and then someone depending on my package hits compile errors attempting to consume my declarations.
Playground Link:
Not possible, depends on generating d.ts declaration files.
Related Issues:
https://github.com/microsoft/TypeScript/issues/35343
https://github.com/microsoft/TypeScript/issues/15559
https://github.com/microsoft/TypeScript/issues/30523
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.