meganrogge / template-string-converter

Autocorrect from quotes to backticks
MIT License
194 stars 24 forks source link

In jsx props, I expect this convert feature #15

Closed ghost closed 4 years ago

ghost commented 4 years ago

Sorry, I couldn't make it a concise title, I couldn't put it into words. But it's easy to express in code.

from

const Hoge = ({ hoge }) => <input value="${hoge}" />

to

const Hoge = ({ hoge }) => <input value={`${hoge}`} />
meganrogge commented 4 years ago

Hi, can you please describe the use case of this more broadly? Is this a specific feature of jsx?

karlhorky commented 4 years ago

So, I believe this request is about supporting the single and double quotes in JSX props (JSX props, which are visually similar to HTML attributes, allow for both types of quotes).

For example, both of the components in this code here below are using hardcoded string values as the value for the name prop:

const Component1 = () => <Greet name="Karl" />
const Component2 = () => <Greet name='Karl' />
const Component1 = () => <Greet name={"Karl"} />
const Component2 = () => <Greet name={'Karl'} />

If they were to be updated to use a ${ inside of the values, such as this step here:

const Component1 = () => <Greet name="${user.name}" />
const Component2 = () => <Greet name='${user.name}' />
const Component1 = () => <Greet name={"${user.name}"} />
const Component2 = () => <Greet name={'${user.name}'} />

...then the Template String Converter VS Code Plugin could convert them to this:

const Component1 = () => <Greet name={`${user.name}`} />
const Component2 = () => <Greet name={`${user.name}`} />
const Component1 = () => <Greet name={`${user.name}`} />
const Component2 = () => <Greet name={`${user.name}`} />

Does this make sense?

meganrogge commented 4 years ago

Thanks for the additional info. This works as is shown below.

example

karlhorky commented 4 years ago

Ah, sorry about this - I made a mistake in my post above! (now updated)

In JSX, if you want to use a template string in a prop, you need to wrap it in curly brackets, like this:

const Component1 = () => <Greet name={`${user.name}`} />
const Component2 = () => <Greet name={`${user.name}`} />

Could this be special-cased for JSX props?

meganrogge commented 4 years ago

So the request is when a user types "${}"in a JSX file, within </>, the quotes are replaced with {` and `} ? This seems like it could have some undesirable consequences. But perhaps I'll add it as a setting that can be enabled.

karlhorky commented 4 years ago

Almost - the curly brackets (to create an embedded expression) should be added only if they don't exist already.

So a pseudo-code algorithm could look like this:

  1. Are we inside an opening tag? (either <...> or <.../>)
  2. Are we in the value of a prop? (eg. we're in between propName=" and " in this code: <tagname propName="..."></tagname>)
  3. Are we inside an embedded expression (the curly brackets) in JSX? (eg. we're inside of propName={" and "})

If 1 and 2 are true but 3 is false, add curly brackets around the backticks. In all other cases, keep the existing behavior.

If this algorithm could run using abstract syntax tree information instead of string matching (so that it only ran on JavaScript files inside of JSX parts), then I guess it could be pretty robust (and also default).


Added some more examples above too: https://github.com/meganrogge/template-string-converter/issues/15#issuecomment-707648750

ghost commented 4 years ago

Thank you karlhorky. you have filled in all the missing pieces.

meganrogge commented 4 years ago

@karlhorky @utadachiren thank you for the feedback and info. I've been pretty busy this week, but hope to get to this sometime next week.

meganrogge commented 4 years ago

alright @karlhorky @utadachiren @capaj try it out and let me know what you think.

karlhorky commented 4 years ago

wowow so fast, thanks @meganrogge! I'll give it a go!

karlhorky commented 4 years ago

Hm... just upgraded to version 0.2.0 and... it doesn't seem to be working (at least with javascript and javascriptreact syntaxes).

Am I doing something wrong?

javascript syntax

Kapture 2020-10-19 at 11 09 36

javascriptreact syntax

Kapture 2020-10-19 at 11 09 58

meganrogge commented 4 years ago

I made it such that this only works when you’re assigning a variable. So try let result= that expression please. The reason for doing this is I want to be sure it’s not firing for non-props. Also, this will only works in JavaScript react/ typescript react files. Lastly, in my matching phrase I didn’t include = as a possible character within the start tag. So I should add that. This should work for your earlier examples above. I’ll update the matching phrase in a bit so that it works for the examples you just showed.

karlhorky commented 4 years ago

I made it such that this only works when you’re assigning a variable. So try let result= that expression please.

hmm... not sure exactly what you mean. Where would I put let result=? This is not a JavaScript expression.

This should work for your earlier examples above

this will only works in JavaScript react/ typescript react files

Ok, I tried writing the following in a javascriptreact file:

const Component1 = () => <Greet name="Karl" />

...and tried adding ${ inside the quotes after ", but this also didn't work (no effect, no changing of quotes).

meganrogge commented 4 years ago

Sorry about this - I’ll investigate. It was working for me last night.

meganrogge commented 4 years ago

recording (7)

@karlhorky it should now work in v 0.2.1. note, I've now made a setting called addBracketsToProps that you have to change from the default value of false to true which controls whether or not this is active instead of file type now.

ghost commented 4 years ago

This is the behavior I was hoping for. thanks @meganrogge @karlhorky

karlhorky commented 4 years ago

Thanks @meganrogge ! It works in some simple cases! 🙌

However, there are still a few bugs. For example:

  1. an element that has a - in the prop value (or, I suppose, probably other characters too?)
  2. multiple props

See in the gif:

Kapture 2020-10-20 at 11 54 34

...or using something else, like using JS Object Spread to put all of the props passed in as arguments:

Screen Shot 2020-10-20 at 11 55 32

meganrogge commented 4 years ago

@karlhorky should now work for all of the above

karlhorky commented 4 years ago

Great, looking really good! 💯 🎉 Thanks @meganrogge !

One last broken case that I can think of is newlines:

function Div(props) {
  return (
    <div
      {...props}
      prop-on-new-line="abc"
      // comment
      another-on-new-line="def"
    />
  );
}

Kapture 2020-10-22 at 10 55 51

meganrogge commented 4 years ago

@karlhorky that's expected. As of now, this extension doesn’t deal with multiline.

karlhorky commented 4 years ago

Ok, I will open a separate issue about this, in case anyone ends up searching for this.

Thanks for your hard work on this!

Edit: open at #20

meganrogge commented 4 years ago

@karlhorky can you pls specify that this is a known issue?

karlhorky commented 4 years ago

Will do, sure!