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

Extract refactoring runs on JSX attribute #802

Closed
automatensalat opened this issue Jan 30, 2023 · 3 comments
Closed

Extract refactoring runs on JSX attribute #802

automatensalat opened this issue Jan 30, 2023 · 3 comments
Labels
🐛 Bug Refactoring breaks existing behavior

Comments

@automatensalat
Copy link
Contributor

Describe the bug

The Extract refactoring can be run on JSX attributes, resulting in invalid syntax:

function MyComponent() {
	return <div id="test">Hello</div>;
}

Run "Extract" on attribute "id", results in

function MyComponent() {
	const extracted = id;
	return <div {extracted}="test">Hello</div>;
}

How to reproduce

See above

Expected behavior

I don't know if there's ever a way this can be useful; I think the refactoring should just show an error message in this case.

Additional information

  • Version of the extension impacted: v6.18.2
@automatensalat automatensalat added the 🐛 Bug Refactoring breaks existing behavior label Jan 30, 2023
@nicoespeon
Copy link
Owner

Thanks for reporting! I handled this case and now it will resolve the closest extractable chunk.

In this case, it will extract the whole JSX Element:

function MyComponent() {
  const extracted = <div id="test">Hello</div>;
  return extracted;
}

@automatensalat
Copy link
Contributor Author

I suppose it could also extract the value of the attribute, i.e.

function MyComponent() {
  const extracted = "test";
  return <div id={extracted}>Hello</div>;
}

Not sure which behavior would be more helpful / expected.

@nicoespeon
Copy link
Owner

Maybe… I went for the consistent behavior of "extract the closest extractable node", I'm not sure how easy it would be to add this one special case at the moment. I think that's fine, you would put your cursor on the value to extract the value and that should work fine 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Refactoring breaks existing behavior
Projects
None yet
Development

No branches or pull requests

2 participants