Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normative: Arbitrary module namespace identifier names #49297

Closed
wants to merge 6 commits into from

Conversation

Jack-Works
Copy link
Contributor

@Jack-Works Jack-Works commented May 29, 2022

Implementing tc39/ecma262#2154

@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label May 29, 2022
@typescript-bot typescript-bot added For Uncommitted Bug PR for untriaged, rejected, closed or missing bug and removed For Milestone Bug PRs that fix a bug with a specific milestone labels Apr 23, 2023
@Jack-Works
Copy link
Contributor Author

Made a full rewrite of this PR. Just get the compiler part to work. Will handle language service days later.

@@ -3415,7 +3420,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
let suggestion: Symbol | undefined;
let suggestedLib: string | undefined;
// Report missing lib first
if (nameArg) {
if (nameArg) {
Copy link
Contributor

@ExE-Boss ExE-Boss Apr 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: extraneous whitespace:

Suggested change
if (nameArg) {
if (nameArg) {

@Jack-Works
Copy link
Contributor Author

Jack-Works commented Jun 14, 2023

Hi, @DanielRosenwasser I have a question. With this PR, it's possible to define members with names that cannot be an identifier on a Symbol.

export const enum InternalSymbolName {
    Call = "__call", // Call signatures
    Constructor = "__constructor", // Constructor implementations
    New = "__new", // Constructor signatures
    Index = "__index", // Index signatures
    ExportStar = "__export", // Module export * declarations
    Global = "__global", // Global self-reference
    Missing = "__missing", // Indicates missing symbol
    Type = "__type", // Anonymous type literal symbol
    Object = "__object", // Anonymous object literal declaration
    JSXAttributes = "__jsxAttributes", // Anonymous JSX attributes object literal declaration
    Class = "__class", // Unnamed class expression
    Function = "__function", // Unnamed function expression
    Computed = "__computed", // Computed property name declaration with dynamic name
    Resolving = "__resolving__", // Indicator symbol used to mark partially resolved type aliases
    ExportEquals = "export=", // Export assignment symbol
    Default = "default", // Default export symbol (technically not wholly internal, but included here for usability)
    This = "this",
}

I wonder if this PR gonna make programmers able to write code that breaks TS by declaring a symbol member that has the same name as an internal symbol, like

export { item as "this" }
// or
export { item as "export=", item as __new }

@DanielRosenwasser
Copy link
Member

I think the comment

Default export symbol (technically not wholly internal, but included here for usability)

is the key here, and the same applies to This. those symbols are just for comparisons in various places. If anything relied on this or default being special entities within a symbol table, they'd probably already be broken.

Maybe @weswigham or @gabritto can back me up on that statement though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants