Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

🐛 noUselessFragments is inconsistent/buggy #3668

Closed
1 task done
nstepien opened this issue Nov 11, 2022 · 0 comments · Fixed by #3858
Closed
1 task done

🐛 noUselessFragments is inconsistent/buggy #3668

nstepien opened this issue Nov 11, 2022 · 0 comments · Fixed by #3858
Assignees
Labels
A-Linter Area: linter L-JSX Language: JSX S-Bug: confirmed Status: report has been confirmed as a valid bug

Comments

@nstepien
Copy link
Contributor

Environment information

CLI:
  Version:              10.0.1
  Color support:        true

Platform:
  CPU Architecture:     x86_64
  OS:                   windows

Environment:
  ROME_LOG_DIR:         unset
  NO_COLOR:             unset
  TERM:                 unset

Rome Configuration:
  Status:               loaded
  Formatter disabled:   true
  Linter disabled:      false

Workspace:
  Open Documents:       0

Discovering running Rome servers...

Running Rome Server: ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

i The client isn't connected to any server but rage discovered this running Rome server.

Server:
  Version:              10.0.1
  Name:                 rome_lsp
  CPU Architecture:     x86_64
  OS:                   windows

Workspace:
  Open Documents:       0

Other Active Server Workspaces:

Workspace:
  Open Documents:       6
  Client Name:          Visual Studio Code
  Client Version:       1.73.1

What happened?

Issue 1

function Component1() {
  const str = 'str';
  return <>{str}</>;
}

function Component2() {
  const str = 'str';
  return (<>{str}</>);
}

The noUselessFragments rule finds an issue in Component2, but not in Component1, the only difference is the parentheses.

Issue 2

const obj = {
  element: <>test</>
};

if I apply the rule suggestion, I end up with this:

const obj = {
  "test"
};

which is not valid JS.

  ! Avoid using unnecessary Fragment.

    176 │ const obj = {
  > 177 │   element: <>test</>
        │            ^^^^^^^^^
    178 │ };
    179 │ 

  i Suggested fix: Remove the Fragment

    175 175 │ 
    176 176 │   const obj = {
    177     │ - ··element:·<>test</>
        177 │ + ··"test"
    178 178 │   };
    179 179 │ 

Expected result

No bugs!

FWIW because of a TypeScript limitation, components cannot return a string for example, otherwise it'll lead to type issues, so some fragments must be preserved in .tsx files.

In the second example, I'm not convinced it's safe to remove the fragment there anyway, we don't know where obj.element will be used.

Code of Conduct

  • I agree to follow Rome's Code of Conduct
@nstepien nstepien added the S-To triage Status: user report of a possible bug that needs to be triaged label Nov 11, 2022
@MichaReiser MichaReiser added S-Bug: confirmed Status: report has been confirmed as a valid bug A-Linter Area: linter L-JSX Language: JSX and removed S-To triage Status: user report of a possible bug that needs to be triaged labels Nov 11, 2022
@ematipico ematipico self-assigned this Nov 14, 2022
@ematipico ematipico added this to the 11.0.0 milestone Nov 14, 2022
@ematipico ematipico removed their assignment Nov 16, 2022
@ematipico ematipico moved this to Todo in Rome 2022 Nov 16, 2022
@ematipico ematipico self-assigned this Nov 17, 2022
Repository owner moved this from Todo to Done in Rome 2022 Nov 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Linter Area: linter L-JSX Language: JSX S-Bug: confirmed Status: report has been confirmed as a valid bug
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants