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

Lookup JSX namespace within factory function #22207

Merged
merged 2 commits into from
Feb 28, 2018

Conversation

weswigham
Copy link
Member

@weswigham weswigham commented Feb 27, 2018

This allows you to mix the types for multiple jsx factories in one compilation in a similar way to how you can now mix factory functions. What this change does is look for an exported JSX namespace under the "react namespace" of the factory function. What this means is that if the factory function is React.createElement, we'll look for React.JSX. If the factory function is p, we'll look for p.JSX. If a local JSX namespace cannot be found, we still fallback to the global one, if present. This does not change how we discover any JSX types beyond looking for them in an additional location.

An an example, react's core exports can now be defined like so:

export function createElement<P>(/*complex types*/): Component<P>;
export namespace createElement {
    export namespace JSX {
        interface Element extends ReactElement<any> {}
        interface ClassElement extends Component<any> {}
        // ... and so on
    }
}

and you get everything you need on the consuming end with your standard

import * as React from "react";
export const MyComponent = (p: {x?: number, y?: number}) => <h1>/*...*/</h1>;

With this change, a JSX package no longer needs to pollute the global scope with types that can conflict with other packages. 😄

Fixes #18131
IMHO, this elegantly solves the aliasing problem posed in the original issue (we're just doing the lookup inside the factory function's namespace, rather than trying to trace back into the file it came from and looking beside it)

@mhegazy
Copy link
Contributor

mhegazy commented Feb 27, 2018

@weswigham can you share perf numbers on one of our RWC jsx code bases comparing to before #21218, to #21218 + this change.

@weswigham
Copy link
Member Author

image
👍 👎?

@@ -15358,16 +15321,27 @@ namespace ts {
return getUnionType(map(instantiatedSignatures, getReturnTypeOfSignature), UnionReduction.Subtype);
}

function getJsxNamespaceForLocation(location: Node) {
Copy link
Member

Choose a reason for hiding this comment

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

Super nit, the naming of getJsxNamespaceForLocation should probably match getJsxStatelessElementTypeAt / getJsxElementClassTypeAt

}

/**
* Returns all the properties of the Jsx.IntrinsicElements interface
*/
function getJsxIntrinsicTagNames(): Symbol[] {
const intrinsics = getJsxType(JsxNames.IntrinsicElements);
function getJsxIntrinsicTagNames(location: Node): Symbol[] {
Copy link
Member

Choose a reason for hiding this comment

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

Similar, getJsxIntrinsicTagNamesAt

@RyanCavanaugh
Copy link
Member

Very nice

@@ -1811,7 +1811,7 @@ declare namespace ts {
getAliasedSymbol(symbol: Symbol): Symbol;
getExportsOfModule(moduleSymbol: Symbol): Symbol[];
getAllAttributesTypeFromJsxOpeningLikeElement(elementNode: JsxOpeningLikeElement): Type | undefined;
getJsxIntrinsicTagNames(): Symbol[];
Copy link
Contributor

Choose a reason for hiding this comment

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

please add a note about this change in the breaking changes section in https://github.com/Microsoft/TypeScript/wiki/API-Breaking-Changes

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. 👍

@mhegazy mhegazy added Breaking Change Would introduce errors in existing code API Relates to the public API for TypeScript labels Feb 28, 2018
@weswigham weswigham merged commit 1a43ad0 into microsoft:master Feb 28, 2018
@weswigham weswigham deleted the local-jsx-namespaces branch February 28, 2018 23:48
@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API Relates to the public API for TypeScript Breaking Change Would introduce errors in existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants