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

Add jsxFactory compiler option. #11267

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10795,13 +10795,24 @@ namespace ts {
checkGrammarJsxElement(node);
checkJsxPreconditions(node);

// The reactNamespace symbol should be marked as 'used' so we don't incorrectly elide its import. And if there
// is no reactNamespace symbol in scope when targeting React emit, we should issue an error.
const reactRefErr = compilerOptions.jsx === JsxEmit.React ? Diagnostics.Cannot_find_name_0 : undefined;
const reactNamespace = compilerOptions.reactNamespace ? compilerOptions.reactNamespace : "React";
const reactSym = resolveName(node.tagName, reactNamespace, SymbolFlags.Value, reactRefErr, reactNamespace);
if (reactSym) {
getSymbolLinks(reactSym).referenced = true;
// The JSX factory namespace symbol should be marked as 'used' so we don't incorrectly elide its import. And if
// it isn't in scope when targeting React emit, we should issue an error.
const jsxFactoryRefErr = compilerOptions.jsx === JsxEmit.React ? Diagnostics.Cannot_find_name_0 : undefined;
let jsxFactoryNamespace = "React";
if (compilerOptions.jsxFactory) {
Copy link
Member

Choose a reason for hiding this comment

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

This could be done once during initializeTypeChecker rather than on every call to checkJsxOpeningLikeElement.

let { entityName } = parseIsolatedEntityName(compilerOptions.jsxFactory);
while (entityName.kind === SyntaxKind.QualifiedName) {
entityName = (<QualifiedName>entityName).left;
}
Debug.assertNode(entityName, node => node.kind === SyntaxKind.Identifier);
Copy link
Member

Choose a reason for hiding this comment

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

There is already an isIdentifier node test you can pass to Debug.assertNode.

jsxFactoryNamespace = (<Identifier>entityName).text;
}
else if (compilerOptions.reactNamespace) {
jsxFactoryNamespace = compilerOptions.reactNamespace;
}
const jsxFactorySym = resolveName(node.tagName, jsxFactoryNamespace, SymbolFlags.Value, jsxFactoryRefErr, jsxFactoryNamespace);
if (jsxFactorySym) {
getSymbolLinks(jsxFactorySym).referenced = true;
}

const targetAttributesType = getJsxElementAttributesType(node);
Expand Down
5 changes: 5 additions & 0 deletions src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ namespace ts {
paramType: Diagnostics.KIND,
description: Diagnostics.Specify_JSX_code_generation_Colon_preserve_or_react,
},
{
name: "jsxFactory",
type: "string",
description: Diagnostics.Specify_the_JSX_factory_function_to_use_when_targeting_react_JSX_emit_e_g_React_createElement_or_h,
},
{
name: "reactNamespace",
type: "string",
Expand Down
12 changes: 12 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2365,6 +2365,14 @@
"category": "Error",
"code": 5066
},
"Option '{0}' can only be used when '{1}' is '{2}'.": {
"category": "Error",
"code": 5067
},
"Invalid value for '{0}'. '{1}' is not a valid identifier/qualified-name.": {
"category": "Error",
"code": 5068
},
"Concatenate and emit output to single file.": {
"category": "Message",
"code": 6001
Expand Down Expand Up @@ -2861,6 +2869,10 @@
"category": "Error",
"code": 6140
},
"Specify the JSX factory function to use when targeting 'react' JSX emit, e.g. 'React.createElement' or 'h'.": {
"category": "Message",
"code": 6141
},
"Variable '{0}' implicitly has an '{1}' type.": {
"category": "Error",
"code": 7005
Expand Down
36 changes: 27 additions & 9 deletions src/compiler/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1618,17 +1618,38 @@ namespace ts {
);
}

function createReactNamespace(reactNamespace: string, parent: JsxOpeningLikeElement) {
export function createEntityName(entityName: string) {
Copy link
Author

Choose a reason for hiding this comment

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

I'm skeptical the implementation of this is ideal, open to suggestions.

Copy link
Member

Choose a reason for hiding this comment

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

@ahejlsberg thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps a parseIsolatedEntityName could set a flag in the parser to have it treat the source as synthetic.

const { entityName: node } = parseIsolatedEntityName(entityName);

// Parsing occurred in an isolated context, so nodes need to be synthesized
// and positions erased for correct positioning in emit.
const synthesize = (node: Node) => {
node.pos = -1;
node.end = -1;
node.flags |= NodeFlags.Synthesized;
forEachChild(node, synthesize);
};
synthesize(node);

return node;
}

export function createJsxFactory(jsxFactory: string) {
const entityName = createEntityName(jsxFactory);
return createExpressionFromEntityName(entityName);
}

export function createReactCreateElement(reactNamespace: string | undefined, parentElement: JsxOpeningLikeElement) {
// To ensure the emit resolver can properly resolve the namespace, we need to
Copy link
Author

Choose a reason for hiding this comment

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

I don't really understand this comment. What does "resolving the namespace" by the emit resolver mean, and why does it need to? My guess would have been to ensure that the React symbol is treated as "used" (so that the import is not elided), but this was already done manually in checker.ts:10804, so I must be missing something.

// treat this identifier as if it were a source tree node by clearing the `Synthesized`
// flag and setting a parent node.
const react = createIdentifier(reactNamespace || "React");
react.flags &= ~NodeFlags.Synthesized;
react.parent = parent;
return react;
react.parent = parentElement;
return createPropertyAccess(react, "createElement");
}

export function createReactCreateElement(reactNamespace: string, tagName: Expression, props: Expression, children: Expression[], parentElement: JsxOpeningLikeElement, location: TextRange): LeftHandSideExpression {
export function createJsxFactoryCall(jsxFactory: Expression, tagName: Expression, props: Expression, children: Expression[], parentElement: JsxOpeningLikeElement, location: TextRange): LeftHandSideExpression {
const argumentsList = [tagName];
if (props) {
argumentsList.push(props);
Expand All @@ -1651,10 +1672,7 @@ namespace ts {
}

return createCall(
createPropertyAccess(
createReactNamespace(reactNamespace, parentElement),
"createElement"
),
jsxFactory,
/*typeArguments*/ undefined,
argumentsList,
location
Expand Down Expand Up @@ -2909,4 +2927,4 @@ namespace ts {
function tryGetModuleNameFromDeclaration(declaration: ImportEqualsDeclaration | ImportDeclaration | ExportDeclaration, host: EmitHost, resolver: EmitResolver, compilerOptions: CompilerOptions) {
return tryGetModuleNameFromFile(resolver.getExternalModuleFileFromDeclaration(declaration), host, compilerOptions);
}
}
}
21 changes: 21 additions & 0 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,13 @@ namespace ts {
return result;
}

/* @internal */
export function parseIsolatedEntityName(content: string) {
const result = Parser.parseIsolatedEntityName(content);
Parser.fixupParentReferences(result.entityName);
Copy link
Member

Choose a reason for hiding this comment

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

Parent references aren't used in the emit transforms with respect to synthesized nodes. If this is only used in emit, then fixing up the parent references is unneeded.

return result;
}

/* @internal */
// Exposed only for testing.
export function parseJSDocTypeExpressionForTests(content: string, start?: number, length?: number) {
Expand Down Expand Up @@ -584,6 +591,20 @@ namespace ts {
return result;
}

export function parseIsolatedEntityName(content: string, allowReservedWords = false) {
Copy link
Member

Choose a reason for hiding this comment

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

Is allowReservedWords ever used by a caller? If not, I would just remove it.

initializeState("file.js", content, ScriptTarget.Latest, /*_syntaxCursor:*/ undefined, ScriptKind.JS);
sourceFile = <SourceFile>{ languageVariant: LanguageVariant.Standard, text: content };

nextToken();
const entityName = parseEntityName(allowReservedWords);
parseExpected(SyntaxKind.EndOfFileToken);

const diagnostics = parseDiagnostics;
Copy link
Member

Choose a reason for hiding this comment

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

Are you using these diagnostics, other than to check that they exist in program.ts:1546? You could return undefined rather than allocating the object literal for every call.

clearState();

return { entityName, diagnostics };
}

function getLanguageVariant(scriptKind: ScriptKind) {
// .tsx and .jsx files are treated as jsx language variant.
return scriptKind === ScriptKind.TSX || scriptKind === ScriptKind.JSX || scriptKind === ScriptKind.JS ? LanguageVariant.JSX : LanguageVariant.Standard;
Expand Down
15 changes: 15 additions & 0 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1533,6 +1533,21 @@ namespace ts {
programDiagnostics.add(createCompilerDiagnostic(Diagnostics.Invalid_value_for_reactNamespace_0_is_not_a_valid_identifier, options.reactNamespace));
}

if (options.jsxFactory) {
if (options.jsx !== JsxEmit.React) {
programDiagnostics.add(createCompilerDiagnostic(Diagnostics.Option_0_can_only_be_used_when_1_is_2, "jsxFactory", "jsx", "react"));
}

if (options.reactNamespace) {
programDiagnostics.add(createCompilerDiagnostic(Diagnostics.Option_0_cannot_be_specified_with_option_1, "jsxFactory", "reactNamespace"));
}

const { diagnostics } = parseIsolatedEntityName(options.jsxFactory);
if (diagnostics.length > 0) {
programDiagnostics.add(createCompilerDiagnostic(Diagnostics.Invalid_value_for_0_1_is_not_a_valid_identifier_Slashqualified_name, "jsxFactory", options.jsxFactory));
}
}

// If the emit is enabled make sure that every output file is unique and not overwriting any of the input files
if (!options.noEmit && !options.suppressOutputPathCheck) {
const emitHost = getEmitHost();
Expand Down
8 changes: 6 additions & 2 deletions src/compiler/transformers/jsx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,12 @@ namespace ts {
|| createAssignHelper(currentSourceFile.externalHelpersModuleName, segments);
}

const element = createReactCreateElement(
compilerOptions.reactNamespace,
const jsxFactory = compilerOptions.jsxFactory
? createJsxFactory(compilerOptions.jsxFactory)
Copy link
Member

Choose a reason for hiding this comment

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

Since this is a fully-synthesized entity name, consider caching it on SourceFile, similar to externalHelpersModuleName rather than reparsing it each time it is hit. You could also create it once at the top of transformJsx.

: createReactCreateElement(compilerOptions.reactNamespace, node);

const element = createJsxFactoryCall(
jsxFactory,
tagName,
objectProperties,
filter(map(children, transformJsxChildToExpression), isDefined),
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2719,6 +2719,7 @@ namespace ts {
inlineSources?: boolean;
isolatedModules?: boolean;
jsx?: JsxEmit;
jsxFactory?: string;
Copy link
Author

Choose a reason for hiding this comment

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

Instead of parsing the jsxFactory to an EntityName multiple times through-out the program, would it be better to have this be EntityName and parse it once up-front?

Copy link
Member

Choose a reason for hiding this comment

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

It'd probably be cleanest to do this once in checker.ts and then only again if needed in emitter.ts.

Copy link
Member

Choose a reason for hiding this comment

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

This is somewhat similar to what we do for externalHelpersModuleName on SourceFile.

lib?: string[];
/*@internal*/listEmittedFiles?: boolean;
/*@internal*/listFiles?: boolean;
Expand Down
4 changes: 4 additions & 0 deletions src/harness/unittests/transpile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,10 @@ var x = 0;`, {
options: { compilerOptions: { jsx: 1 }, fileName: "input.js", reportDiagnostics: true }
});

transpilesCorrectly("Supports setting 'jsxFactory'", "x;", {
options: { compilerOptions: { jsxFactory: "React.createElement", jsx: JsxEmit.React }, fileName: "input.js", reportDiagnostics: true }
});

transpilesCorrectly("Supports setting 'lib'", "x;", {
options: { compilerOptions: { lib: ["es2015", "dom"] }, fileName: "input.js", reportDiagnostics: true }
});
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions tests/baselines/reference/tsxReactEmitJsxFactory1.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
tests/cases/conformance/jsx/file.tsx(8,2): error TS2304: Cannot find name 'h'.


==== tests/cases/conformance/jsx/file.tsx (1 errors) ====
declare module JSX {
interface IntrinsicElements {
[s: string]: any;
}
}

// This should issue an error as 'h' is not declared.
<div />;
~~~
!!! error TS2304: Cannot find name 'h'.

14 changes: 14 additions & 0 deletions tests/baselines/reference/tsxReactEmitJsxFactory1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//// [file.tsx]
declare module JSX {
interface IntrinsicElements {
[s: string]: any;
}
}

// This should issue an error as 'h' is not declared.
<div />;


//// [file.js]
// This should issue an error as 'h' is not declared.
h("div", null);
14 changes: 14 additions & 0 deletions tests/baselines/reference/tsxReactEmitJsxFactory2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//// [file.tsx]
declare module JSX {
interface IntrinsicElements {
[s: string]: any;
}
}

declare var h: any;

<div />;


//// [file.js]
h("div", null);
18 changes: 18 additions & 0 deletions tests/baselines/reference/tsxReactEmitJsxFactory2.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
=== tests/cases/conformance/jsx/file.tsx ===
declare module JSX {
>JSX : Symbol(JSX, Decl(file.tsx, 0, 0))

interface IntrinsicElements {
>IntrinsicElements : Symbol(IntrinsicElements, Decl(file.tsx, 0, 20))

[s: string]: any;
>s : Symbol(s, Decl(file.tsx, 2, 3))
}
}

declare var h: any;
>h : Symbol(h, Decl(file.tsx, 6, 11))

<div />;
>div : Symbol(JSX.IntrinsicElements, Decl(file.tsx, 0, 20))

19 changes: 19 additions & 0 deletions tests/baselines/reference/tsxReactEmitJsxFactory2.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
=== tests/cases/conformance/jsx/file.tsx ===
declare module JSX {
>JSX : any

interface IntrinsicElements {
>IntrinsicElements : IntrinsicElements

[s: string]: any;
>s : string
}
}

declare var h: any;
>h : any

<div />;
><div /> : any
>div : any

14 changes: 14 additions & 0 deletions tests/baselines/reference/tsxReactEmitJsxFactory3.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//// [file.tsx]
declare module JSX {
interface IntrinsicElements {
[s: string]: any;
}
}

declare var React: any;

<div />;


//// [file.js]
React.createElement("div", null);
18 changes: 18 additions & 0 deletions tests/baselines/reference/tsxReactEmitJsxFactory3.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
=== tests/cases/conformance/jsx/file.tsx ===
declare module JSX {
>JSX : Symbol(JSX, Decl(file.tsx, 0, 0))

interface IntrinsicElements {
>IntrinsicElements : Symbol(IntrinsicElements, Decl(file.tsx, 0, 20))

[s: string]: any;
>s : Symbol(s, Decl(file.tsx, 2, 3))
}
}

declare var React: any;
>React : Symbol(React, Decl(file.tsx, 6, 11))

<div />;
>div : Symbol(JSX.IntrinsicElements, Decl(file.tsx, 0, 20))

19 changes: 19 additions & 0 deletions tests/baselines/reference/tsxReactEmitJsxFactory3.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
=== tests/cases/conformance/jsx/file.tsx ===
declare module JSX {
>JSX : any

interface IntrinsicElements {
>IntrinsicElements : IntrinsicElements

[s: string]: any;
>s : string
}
}

declare var React: any;
>React : any

<div />;
><div /> : any
>div : any

14 changes: 14 additions & 0 deletions tests/baselines/reference/tsxReactEmitJsxFactory4.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//// [file.tsx]
declare module JSX {
interface IntrinsicElements {
[s: string]: any;
}
}

declare var a: any;

<div />;


//// [file.js]
a.b.c("div", null);
Loading