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-component-definition breaks code #2765

Closed
wmertens opened this issue Aug 23, 2020 · 6 comments · Fixed by #2771
Closed

function-component-definition breaks code #2765

wmertens opened this issue Aug 23, 2020 · 6 comments · Fixed by #2771

Comments

@wmertens
Copy link

When I auto-fix function-component-definition, it decides that the below is a function component (it's not really) and that it should use arrow notation:

const RULES = [
	{
		serialize(el) {
			if (el.type !== 'tag') return
			const data = el.data.toJS ? el.data.toJS() : el.data
			const text = el.nodes[0].text

			return <Tag linked value={{...data, props: {label: text}}} />
		},
	},
]

It proceeds to auto-fix like this:

const RULES = [
	{
		serialize(el) => {
[...]

which is invalid. It should have changed it to serialize: el => { instead

Another option is that the rule ignores object function notation.

@ljharb
Copy link
Member

ljharb commented Aug 23, 2020

This seems like two issues - one is that the autofix is incorrect, and produces invalid syntax. The other is that since this is a lowercase method of an object literal, we shouldn't detect it as a component.

@ljharb
Copy link
Member

ljharb commented Aug 23, 2020

cc @stefan-wullems

@stefan-wullems
Copy link
Contributor

I'll take a look

@ljharb
Copy link
Member

ljharb commented Aug 23, 2020

(to clarify; the invalid autofix is a much more urgent issue)

@wmertens
Copy link
Author

Great, thanks! Question: does it generally ignore lowercase identifiers now or is that a separate bug?

@ljharb
Copy link
Member

ljharb commented Aug 30, 2020

Yes, custom components must be PascalCased as far as I know.

If there's a case you think is a bug, please do file an issue :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants