Typescript: TS3.7 regression: renamed exports in namespaces do not resolve in nested namespaces

Created on 21 Oct 2019  ·  7Comments  ·  Source: microsoft/TypeScript

TypeScript Version: 3.7.0-beta

Search Terms: namespace renamed export

Code

export declare const x: number;
declare global {
  namespace clutz {
    export {x as some$mangled$name$x};
  }
  namespace clutz.some$mangled$name {
    export {some$mangled$name$x as x};
  }
}

(Using the mangled alias name some$mangled$name$x instead of x makes sure that no other symbol in the namespace accidentally shadows the outer x.)

Expected behavior:

In TypeScript 3.6, some$mangled$name$x within the nested namespace clutz.some$mangled$name resolves to export {x as clutz.some$mangled$name$x} in the parent namespace, and thus eventually to x.

Actual behavior:

In TypeScript 3.7, some$mangled$name$x does not resolve in the nested namespace.

Playground Link: (no 3.7 playground around)

Related Issues:

Working as Intended

Most helpful comment

To clarify from our perspective, I think this is easy to fix for us, but I
wanted to call out the regression and see if it was intentional. So wontfix
is ok :-)

All 7 comments

Looked into this but not entirely sure of the right answer.

It looks like namespaces were brought to act a bit more like how es modules work in the allowJs + declarations PR here making it so that code like this:

declare namespace clutz {
    var x: string

    export {x as some$mangled$name$x};
    var y: typeof some$mangled$name$x;
  }

3.6 works | 3.7 fails

Which is also the same as how the lookups across the cascading namespaces. The underlaying question is should the names of named export declarations be visible across namespaces in a way that's different from es modules? e.g. should this be allowed in a namespace but not an esmodule:

const x = 3;
export { x as foo };
export { foo as bar };

/cc @weswigham @RyanCavanaugh

Mmmm, so the key here is that declare module "..." and actual module files need to behave the same way (so declarations for outFile work as expected). Ambient namespaces getting caught up in the change is _probably_ unintentional. If the original change is restricted to just external module declarations, I _think_ it's OK? The tests should show if it's not. The JS declaration emitter runs up against, like, _all_ the quirks of name resolution, so hopefully the examples and tests should show what's needed.

@weswigham I’m not sure if I interpreted your suggestion correctly, but we tried this:

- if (location.kind === SyntaxKind.SourceFile || (isModuleDeclaration(location) && location.flags & NodeFlags.Ambient && !isGlobalScopeAugmentation(location))) {
+ if (location.kind === SyntaxKind.SourceFile || (isNonGlobalAmbientModule(location) && location.flags & NodeFlags.Ambient)) {

And a bunch of JS declaration emit tests fail, like this one:

// @allowJs: true
// @checkJs: true
// @target: es5
// @outDir: ./out
// @declaration: true
// @filename: bar.js
class Bar {}
module.exports = Bar;
// @filename: cls.js
const Bar = require("./bar");
const Strings = {
    a: "A",
    b: "B"
};
class Foo extends Bar {}
module.exports = Foo;
module.exports.Strings = Strings;

which produces the declarations

export = Foo;
declare const Foo_base: typeof import("./bar");
declare class Foo extends Foo_base {
}
declare namespace Foo {
    export { Strings };
}
declare namespace Strings {
    export const a: string;
    export const b: string;
}

and our change produces the error TS2303: Circular definition of import alias 'Strings'.. It seems like ambient namespaces _do_ need to behave the same way as ES modules here.

Yeah, ok, that was definitely the motivator for the change. You _really_ don't want export {Thing} to be circular within a namespace, as that's.... rather useless.

To me, it definitely feels like the new behavior is good/expected behavior. I never expect export { x as y } to introduce a locally visible name y. It feels weird to treat that differently in ambient namespaces and real modules.

To clarify from our perspective, I think this is easy to fix for us, but I
wanted to call out the regression and see if it was intentional. So wontfix
is ok :-)

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Was this page helpful?
0 / 5 - 0 ratings