Fix source type detection when parsing TypeScript#17107
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/58671 |
| } | ||
| this.parseExportFrom(node, true); | ||
|
|
||
| if (node.exportKind !== "type") { |
There was a problem hiding this comment.
Here is one thing typescript-eslint differs: they set the sourceType to module even when the source only includes type imports/exports: AFAIK TS does not offer ways to include types via commonJS require, so I am not convinced that we should do so as well. To align with their AST we can monkey patch the eslint-parser instead.
@bradzacher Could you shred some light here? Thank you.
There was a problem hiding this comment.
It's actually entirely correct to treat the file as a module if it only contains only type-only import -- it's the same behaviour that TS itself exhibits.
For example you can see in this playground.
- the emitted JS code is
export {}; - there is no error on the
Mapinterface (see below)
But if you comment out the type-only import then:
- the emitted JS code is instead
"use strict";as it's no longer treated as an ESM file - there is an error on the
Mapinterface as TS is trying to declaration merge with the global type.
So yeah -- our behaviour is aligned with TS itself.
There was a problem hiding this comment.
AFAIK TS does not offer ways to include types via commonJS require
It does, but not in the same "type only" fashion.
// module.ts
namespace Foo {
export type T = number;
}
export = Foo;
// other.ts
import Foo = require('./module');
type T = Foo.T;It's much more cumbersome, but still valid.
There was a problem hiding this comment.
@bradzacher Thank you for your detailed explanation, from which I have learned a lot. In that case I think we will align to TS as well.
| "type": "Program", | ||
| "start":0,"end":78,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":3,"column":25,"index":78}}, | ||
| "sourceType": "script", | ||
| "sourceType": "module", |
There was a problem hiding this comment.
This PR also changes the flow handling. As we can see from the playground, both
import react from "react";
const yield = 1;and
import type react from "react";
const yield = 1;are accepted, which means flow does not set the script type based on imports. So we already deviated from the flow behaviour for non-type imports.
* add new test cases * fix: move sawUnambiguousESM to parseExport * fix: set sawUnambiguousESM on type imports * update test fixtures
export { a }withtypescriptplugin andunambiguoussource type does not set program sourceType tomodule(part of #16679)This issue surfaces when we are aligning to the typescript-eslint AST. The typescript parser plugin called
super.parseExportwithout handling thesawUnambiguousESMstate, here we moved the state handling intoparseExportso it should work for both JS and TS.