Skip to content

Version 5.0.4 -> 5.1.3 does not convert tsx element when element is deconstructed behind condition #54798

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

Closed
jscott-resilient opened this issue Jun 27, 2023 · 2 comments
Labels
Duplicate An existing issue was already created

Comments

@jscott-resilient
Copy link

Bug Report

Hello Typescript team!

I found this bug when changing versions from 5.0.4 to 5.1.3. I have a work around but thought I'd let you know!

🔎 Search Terms

webpack typescript v5.1.3 react

🕗 Version & Regression Information

  • No crash occurs, only when webpack parses the converted typescript (javascript) files did I spot this bug
  • This changed between versions 5.0.4 and 5.1.3

💻 Code

The following code would not convert the 'html' behind the deconstructed conditional object to javascript (i.e. React.createElement)

<Accordion
      label={
        <AccordionLabel
          heading={'Names'}
          info={`${routingTableChangesSummary.nameChanges} name${routingTableChangesSummary.nameChanges > 1 ? 's' : ''} changed`}
        />
      }
      {...(routingTableChangesSummary.nameChangesDetails
        ? {
            expandedContent: (
              <ChangeDetailsTable
                totalChanges={routingTableChangesSummary.nameChanges}
                changeDetails={routingTableChangesSummary.nameChangesDetails}
                emptyCellText="(empty)"
              />
            )
          }
        : undefined)}
    />

🙁 Actual behavior

The code snippet above would convert to:

React.createElement(Accordion, Object.assign({ label: React.createElement(AccordionLabel, { heading: 'Names', info: `${routingTableChangesSummary.nameChanges} name${routingTableChangesSummary.nameChanges > 1 ? 's' : ''} changed` }) }, (routingTableChangesSummary.nameChangesDetails
                ? {
                    expandedContent: (<ChangeDetailsTable totalChanges={routingTableChangesSummary.nameChanges} changeDetails={routingTableChangesSummary.nameChangesDetails} emptyCellText="(empty)"/>)
                }
                : undefined)))

You can see it leaves the ChangeDetailsTable as html

🙂 Expected behavior

It should convert to the code below (it did in 5.0.4 but changed in 5.1.3):

React.createElement(Accordion, Object.assign({ label: React.createElement(AccordionLabel, { heading: 'Names', info: `${routingTableChangesSummary.nameChanges} name${routingTableChangesSummary.nameChanges > 1 ? 's' : ''} changed` }) }, (routingTableChangesSummary.nameChangesDetails
                ? {
                    expandedContent: (React.createElement(ChangeDetailsTable, { totalChanges: routingTableChangesSummary.nameChanges, changeDetails: routingTableChangesSummary.nameChangesDetails, emptyCellText: "(empty)" }))
                }
                : undefined)))

Work around

Replacing the use of the deconstructor ...({}) with && allowed typescript to pick up the html correctly.

<Accordion
      label={
        <AccordionLabel
          heading={'Names'}
          info={`${routingTableChangesSummary.nameChanges} name${routingTableChangesSummary.nameChanges > 1 ? 's' : ''} changed`}
        />
      }
      expandedContent={
        routingTableChangesSummary.nameChangesDetails && (
          <ChangeDetailsTable
            totalChanges={routingTableChangesSummary.nameChanges}
            changeDetails={routingTableChangesSummary.nameChangesDetails}
            emptyCellText="(empty)"
          />
        )
      }
    />

Honestly this is a cleaner way to code this so we'll be continuing with our change, probably low priority but again might as well let you know. If you need anything else just drop me a reply and I'll get back to you when I can!

Thanks for your time,

Joe

@nmain
Copy link

nmain commented Jun 27, 2023

Duplicate of #54411.

My search terms were jsx in:title.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Jun 27, 2023
@typescript-bot
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants