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

function-paren-newline error vs. max-len error #1584

Closed
blord-fullscreen opened this issue Oct 10, 2017 · 14 comments
Closed

function-paren-newline error vs. max-len error #1584

blord-fullscreen opened this issue Oct 10, 2017 · 14 comments

Comments

@blord-fullscreen
Copy link

The snippet:

export default connect(mapStateToProps, mapDispatchToProps)(
  generateDndContextWithBackend(SiteEditorContainer));

gives me the error Unexpected newline after "("

But when I remove the newline:

export default connect(mapStateToProps, mapDispatchToProps)(generateDndContextWithBackend(SiteEditorContainer));

I get the max-len error, because the line is longer than 100 characters.

What's the way to fix this to avoid all errors? function-paren-newline isn't currently smart enough to take account of line length, which seems like a shortcoming, especially now that we have highly evolved line-length tools like Prettier. I understand that there are tradeoffs, and best practices may conflict, but in principle there should be an error-free way to write this code that isn't highly contorted. Solution?

@yutin1987
Copy link

yutin1987 commented Oct 10, 2017

I have the same issue, happened at function-paren-newline

bad

    const wrapper = shallow(
      <Tabbar active={0}>
        <Tabbar.Tab>Teb 1</Tabbar.Tab>
        <Tabbar.Tab>Teb 2</Tabbar.Tab>
      </Tabbar>,
    );

bad

const wrapper = shallow(<Tabbar active={0}>
      <Tabbar.Tab>Teb 1</Tabbar.Tab>
      <Tabbar.Tab>Teb 2</Tabbar.Tab>
                            </Tabbar>)

const wrapper = shallow(<Tabbar active={0}>
      <Tabbar.Tab>Teb 1</Tabbar.Tab>
      <Tabbar.Tab>Teb 2</Tabbar.Tab>
    </Tabbar>);

react/jsx-indent vs react/jsx-closing-bracket-location on end line

@blord-fullscreen
Copy link
Author

blord-fullscreen commented Oct 10, 2017

@yutin1987 That is actually a different problem. None of your lines above look too long. Perhaps something is odd with your config. Try pasting into the Prettier playground.

@ljharb
Copy link
Collaborator

ljharb commented Oct 10, 2017

Prettier has nothing to do with this; this guide does not endorse or recommend using Prettier.

@yutin1987 what about this:

export default connect(mapStateToProps, mapDispatchToProps)(
  generateDndContextWithBackend(SiteEditorContainer),
);

@blord-fullscreen
Copy link
Author

@ljharb That doesn't work. In fact, it doubles the error:

Unexpected newline after "("
Unexpected newline before ")"

@yutin1987
Copy link

yutin1987 commented Oct 11, 2017

@ljharb @blord-fullscreen thank you for your answer.

because it is too long, the maximum line length of 100. (max-len)

const wrapper = shallow(<Tabbar active={0}><Tabbar.Tab>Teb 1</Tabbar.Tab><Tabbar.Tab>Teb 2</Tabbar.Tab></Tabbar>);

so, I need multiline. when I use enzyme shallow to testing.

    // exceeds the maximum line length of 100. (max-len)
    shallow(<Tabbar active={0}><Tabbar.Tab>Teb 1</Tabbar.Tab><Tabbar.Tab>Teb 2</Tabbar.Tab></Tabbar>);

    // Closing tag of a multiline JSX expression must be on its own line. (react/jsx-closing-tag-location)
    shallow(<Tabbar active={0}>
      <Tabbar.Tab>Teb 1</Tabbar.Tab><Tabbar.Tab>Teb 2</Tabbar.Tab></Tabbar>);

    // Expected indentation of 4 space characters but found 12. (react/jsx-indent)
    shallow(<Tabbar active={0}>
      <Tabbar.Tab>Teb 1</Tabbar.Tab><Tabbar.Tab>Teb 2</Tabbar.Tab>
            </Tabbar>);

    // Expected closing tag to match indentation of opening. (react/jsx-closing-tag-location)
    shallow(<Tabbar active={0}>
      <Tabbar.Tab>Teb 1</Tabbar.Tab><Tabbar.Tab>Teb 2</Tabbar.Tab>
    </Tabbar>);

    // Unexpected newline after '('. (function-paren-newline)
    // Unexpected newline before ')'. (function-paren-newline)
    shallow(
      <Tabbar active={0}>
        <Tabbar.Tab>Teb 1</Tabbar.Tab><Tabbar.Tab>Teb 2</Tabbar.Tab>
      </Tabbar>
    );

@ljharb
Copy link
Collaborator

ljharb commented Oct 11, 2017

This seems like it's actually a bug in the function-paren-newline rule (or an unavoidable conflict with max-len).

For now, you may want to disable that rule entirely; I'll leave this open to investigate disabling it in the config itself.

@the-noob
Copy link

the-noob commented Nov 2, 2017

Hopefully I'm not wrong but you are using "multiline" (default) so this should work ?

const isLocalhost = Boolean(
  window.location.hostname === 'localhost' ||
  // [::1] is the IPv6 localhost address.
  window.location.hostname === '[::1]' ||
  // 127.0.0.1/8 is considered localhost for IPv4.
  window.location.hostname.match(
    /^127(?:\.(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)){3}$/,
  ),
);

Also posted in the ESLint issue

@ljharb
Copy link
Collaborator

ljharb commented Nov 2, 2017

@yutin1987 fwiw, this is how we handle your example:

shallow((
  <Tabbar active={0}>
    <Tabbar.Tab>Teb 1</Tabbar.Tab><Tabbar.Tab>Teb 2</Tabbar.Tab>
  </Tabbar>
));

or

shallow(
  <Tabbar active={0}>
    <Tabbar.Tab>Teb 1</Tabbar.Tab><Tabbar.Tab>Teb 2</Tabbar.Tab>
  </Tabbar>,
);

@brandonburkett
Copy link

The second option given still gives me the error. Including the training comma still complains about Unexpected newline after '('. (function-paren-newline)

<FormItem label="Full Name">
    {getFieldDecorator(fullName.id, fullName.options)(
        <Input type="text" size="large" placeholder="First & Last Name" />,
    )}
</FormItem>

@ljharb
Copy link
Collaborator

ljharb commented Dec 10, 2017

@devourment77

<FormItem label="Full Name">
    {getFieldDecorator(fullName.id, fullName.options)((
        <Input type="text" size="large" placeholder="First & Last Name" />
    ))}
</FormItem>

Reggino added a commit to kingsquare/eslint-config-kingsquare that referenced this issue Mar 15, 2018
@trevtrich
Copy link

We run into this as well. For context, we use multiline. When we have situations like:

await Promise.all(response.list.map(blob => getResource({dataBlobId: blob.id, att: somethingElse})));

We hit our line length limit. Converting that to:

await Promise.all(
     response.list.map(blob => getBlobResource({dataBlobId: blob.id, att: somethingElse}))
);

is not allowed, though i think that is the cleanest way to handle this one. Other variations that do pass are:

await Promise.all(response.list.map(blob => getBlobResource({
    dataBlobId: blob.id,
    att: somethingElse
})));

await Promise.all(response.list.map(blob =>
    getBlobResource({dataBlobId: blob.id, att: somethingElse})));

both of which I feel are less clean than the first.

@trevtrich
Copy link

trevtrich commented May 17, 2018

Also worth noting that we could additionally use the following (notice the additional parens) similar to what @ljharb mentioned for handling JSX code, which we also use right now because of this:

await Promise.all((
     response.list.map(blob => getBlobResource({dataBlobId: blob.id, att: somethingElse}))
));

Though this is close, it feels wrong to add in otherwise unnecessary code to pass the linting.

@ljharb
Copy link
Collaborator

ljharb commented May 17, 2018

I consider it as the multiline jsx is one unit, wrapped in parens - and that’s passed into a function.

@ljharb
Copy link
Collaborator

ljharb commented Aug 10, 2019

Closing; happy to reopen if there's something actionable here.

@ljharb ljharb closed this as completed Aug 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants