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

Rule proposal: jsx-tag-spacing #693

Closed
Jessidhia opened this issue Jul 20, 2016 · 7 comments
Closed

Rule proposal: jsx-tag-spacing #693

Jessidhia opened this issue Jul 20, 2016 · 7 comments

Comments

@Jessidhia
Copy link
Contributor

Jessidhia commented Jul 20, 2016

For some reason, / > is valid JSX, as is / followed by any kind of whitespace until the >. This rule would require that the self-closing terminator is always written as an atomic />.

Not having a check for this also allows for funny workarounds for jsx-closing-bracket-location, such as this (when using after-props):

<SomeComponent
  with='many'
  props='specified' /
>

Update: #693 (comment)

@ljharb
Copy link
Member

ljharb commented Jul 20, 2016

I'm a strong +1 on this - there should never be a space between / and > and I'm horrified that someone would ever come up with that.

Jessidhia pushed a commit to Jessidhia/eslint-plugin-react that referenced this issue Jul 25, 2016
@Jessidhia
Copy link
Contributor Author

It also turns out that <Tag>< /Tag> is weird but valid JSX too, so I made the rule also check for this.

@ljharb
Copy link
Member

ljharb commented Jul 25, 2016

Also </ Tag> ideally.

@Jessidhia
Copy link
Contributor Author

I can implement that, but I think it should be a separate rule. This one concerns itself with space in the opener/closer itself, while the other rule would worry about whitespace between the opener and the component name (analogous to jsx-space-before-closing).

@Jessidhia
Copy link
Contributor Author

Jessidhia commented Jul 25, 2016

Per discussion with @ljharb on the babel slack:

Merge this proposed rule with jsx-space-before-closing and a rule for checking space after opening, with a default schema like:

{
  closingSlash: 'never', // internal space between the "</" and "/>" tokens, e.g. "< /" would warn
  beforeClosing: 'always', // same thing as the current jsx-space-before-closing option
  afterOpening: 'never', // same thing as jsx-space-before-closing, but right after the "<" or "</"
}

afterOpening would also have an 'allow-multiline' suboption. Adding an 'allow-multiline' suboption to beforeClosing (and using it as default instead) is possible, but a stricter beforeClosing: 'always' would conflict with jsx-closing-bracket-location.

Each suboption would have an 'allow' suboption to disable the respective check.

@ljharb
Copy link
Member

ljharb commented Jul 25, 2016

Let's also rename this rule to jsx-tag-spacing - and then we can deprecate jsx-space-before-closing and copy the tests over.

@Jessidhia Jessidhia changed the title Rule proposal: enforce that the self-closing terminator is written as a single token Rule proposal: jsx-tag-spacing Jul 25, 2016
@Jessidhia
Copy link
Contributor Author

Jessidhia commented Jul 25, 2016

Because jsx-space-before-closing only works on self-closing tags, I'll name the equivalent option beforeSelfClosing instead. A beforeClosing that applies to regular tags would conflict with jsx-closing-bracket-location.

Jessidhia pushed a commit to Jessidhia/eslint-plugin-react that referenced this issue Jul 25, 2016
Jessidhia pushed a commit to Jessidhia/eslint-plugin-react that referenced this issue Jul 26, 2016
Jessidhia pushed a commit to Jessidhia/eslint-plugin-react that referenced this issue Aug 25, 2016
Jessidhia pushed a commit to Jessidhia/eslint-plugin-react that referenced this issue Aug 29, 2016
Jessidhia pushed a commit to Jessidhia/eslint-plugin-react that referenced this issue Oct 24, 2016
@ljharb ljharb closed this as completed in 3b5063f Nov 7, 2016
ljharb added a commit that referenced this issue Nov 7, 2016
[new] Add jsx-tag-spacing rule (Fixes #693)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants