fix(compiler-cli): allow import of modules ending with .js, .ts, etc. #14664
Conversation
|
What scenario does this enable? |
|
When we're importing a symbol from a package which name ends with a string which matches the Here's an example which uses the package called import { Http } from '@angular/http';
import { Observable } from 'rxjs/Observable';
// Doesn't work because AoT host removes the
// ".js$" from the imported library, which
// causes TypeScript to try to look for the
// package "aspect" instead of "aspect.js".
import { Wove } from 'aspect.js';
// Works with this line because once the AoT host
// drops ".js$" from "aspect.js/aspect.js", the
// TypeScript compiler is able to resolve the file
// "node_modules/aspect.js/aspect.js"
// import { Wove } from 'aspect.js/aspect.js';
@Wove()
export class NameListService {
constructor(private http: Http) {}
get(): Observable<string[]> {
return this.http.get('assets/data.json')
.map((res: Response) => res.json());
}
}Without the change the As an attachment I'm providing a demo which can reproduce the problem following the steps: In contrast: works because there's not AoT compilation involved. |
|
I see. This needs to be limited to v4.0 as we still need to support 1.8 for v2.0 which does not support this. Also, please investigate the e2e failure. |
|
I will take a look at the e2e failure during the weekend.
Does this mean I need to open the PR against different branch? |
|
No I just need to make a note to the caretaker not to cherry-pick this to v2.0. When this is ready I will take care of it. |
|
Do we agree that in this case, the compiler will first search for |
|
|
|
After looking into the e2e failure, I found that the TypeScript module resolution algorithms treats files with According to the current behavior, The current behavior seems compatible with TypeScript 1.8 as well. |
Fix #14663 Allow importing node modules which name ends with the strings: ".ts", ".d", ".ts", ".js", ".jsx", ".tsx". According to microsoft/TypeScript#4595 TypeScript knows how to handle imports with an extension so relative imports should not be a problem (as shown in the tests). If you prefer to keep the replace, another, alternative solution (less clean I suppose) is: ```ts moduleNameToFileName(m: string, containingFile: string): string|null { // ... const original = m; m = m.replace(EXT, ''); const resolve = (m: string) => ts.resolveModuleName( m, containingFile.replace(/\\/g, '/'), this.options, this.resolveModuleNameHost) .resolvedModule; let resolved = resolve(m); if (!resolved) { resolved = resolve(original); } return resolved ? this.getCanonicalFileName(resolved.resolvedFileName) : null; }; ```
|
What is the status of this PR ? |
|
Last time I tried |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fix #14663
Allow importing node modules which name ends with the strings:
.ts,.d,.ts,.js,.jsx,.tsx.According to microsoft/TypeScript#4595
TypeScript knows how to handle imports with an extension so relative
imports should not be a problem (as shown in the tests).
If you prefer to keep the replace, another, alternative solution (less clean I suppose) is:
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x")
What is the current behavior? (You can also link to an open issue here)
AoT doesn't allows import of package names ending with
.js,.ts, etc.What is the new behavior?
AoT allows import of package names ending with
.js,.ts, etc.Does this PR introduce a breaking change? (check one with "x")