Skip to content

Conversation

@Andarist
Copy link
Contributor

Please verify that:

  • There is an associated issue in the Backlog milestone (required)
  • Code is up-to-date with the master branch
  • You've successfully run gulp runtests locally
  • There are new or updated unit tests validating the change

Fixes #41146

cc @gaearon @lunaruan @weswigham

I've first added a commit with tests covering the cases around key props and the new JSX transform as those were not covered by any tests I could find (no tests for this were introduced in #39199) and then I've implemented a fix that auto-inserts import for createElement from the jsxImportSource itself so it should be nicely visible what actual change this PR brings.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Oct 17, 2020
}

export function createExpressionForJsxElement(factory: NodeFactory, jsxFactoryEntity: EntityName | undefined, reactNamespace: string, tagName: Expression, props: Expression | undefined, children: readonly Expression[] | undefined, parentElement: JsxOpeningLikeElement, location: TextRange): LeftHandSideExpression {
export function createExpressionForJsxElement(factory: NodeFactory, callee: Expression, tagName: Expression, props: Expression | undefined, children: readonly Expression[] | undefined, location: TextRange): LeftHandSideExpression {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the signature differs now from the accompanying createExpressionForJsxFragment but this is just a util file so I don't think it's particularly important but let me know if you'd like this to be refactored somehow.


function getImplicitImportForName(name: string) {
const existing = currentFileState.utilizedImplicitRuntimeImports?.get(name);
const importSource = name === "createElement"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a special case for the createElement within this "generic" util, but in the name of KISS I think it's perfectly OK here. The transform is tied to the semantics defined by React anyhow, this won't change often and there is near-zero chance that createElement will get reintroduced in the future in some other entrypoint in JSX runtimes

Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

I pushed a merge for the conflicts introduced this morning, and plan to merge once CI is green, thanks!

@weswigham weswigham merged commit 8b728c8 into microsoft:master Oct 20, 2020
@Andarist Andarist deleted the fix/create-element-new-jsx-transform branch October 20, 2020 07:35
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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.

[jsx-runtime] createElement fallback shoud from @jsxImportSource

3 participants