search

oppia / oppia

A free, online learning platform to make quality education accessible for all.
https://www.oppia.org
Apache License 2.0
5.63k stars 3.78k forks source link

[BUG]: Prettier linter does not allow escaping characters in strings. #20199

Closed jnvtnguyen closed 2 weeks ago

jnvtnguyen commented 3 weeks ago

Describe the bug

When I try to escape certain characters in a string, when committing, prettier seems to remove these escaping characters, preventing some of the code from working.

URL of the page where the issue is observed.

N/A

Steps To Reproduce

  1. Try to escape certain characters in a string with backslash ().
  2. Add your changes to git and commit.

Expected Behavior

I expect prettier to not remove these escaping characters.

Screenshots/Videos

https://github.com/oppia/oppia/assets/70992422/cfed7cba-a3cd-4ad1-8cc9-e4813cc425d5

What device are you using?

Desktop

Operating System

Windows

What browsers are you seeing the problem on?

No response

Browser version

No response

Additional context

This seems to have also affected existing code where certain characters in strings are escaped, when we did the whole prettify on the codebase. Only have observed on regexes.

Tips for developers

Before addressing the bug, please identify which PR caused the issue (you can follow the steps here). If you identify the PR, comment on the issue with a link to it. If not, mention the commit hash of the oldest commit you saw the bug on (and the month and year it was made in).

Then, please leave a comment with details of the approach that you plan to take to fix the issue (see example).

Note: If this is your first Oppia issue, please make sure to follow our guidelines for choosing an issue and setting things up. You will also need to show a demo of the fix working correctly on your local machine. Thanks!

seanlip commented 2 weeks ago

@jnvtnguyen Thanks for filing. FYI the linter is called Prettier not Prettify :)

Is this info useful for addressing it? https://stackoverflow.com/questions/46878271/prettier-auto-correct-regex-escaping-forward-slash

jnvtnguyen commented 2 weeks ago

@seanlip Ah thanks, messed up the name. I was able to use regular regexes, though still don't know if this should be intended behavior as it also removes backslashes from regular strings.

seanlip commented 2 weeks ago

@jnvtnguyen Can you give a non-regex example where this matters (i.e. the removal breaks things)?

jnvtnguyen commented 2 weeks ago

@seanlip The only place I can think of is ' in ' so \', but it seems that prettier automatically changes single quotes to double quotes if that happens. So I guess its fine, other than that I can just change the RegExp({string with backslashes}) to raw regexps. (though using strings does help with readability since you can concatenate).

seanlip commented 2 weeks ago

@jnvtnguyen Understood, yeah. But probably the benefit of using a simple vanilla configuration trumps that, so I'd suggest that we go with the defaults for now unless this becomes a recurring pain. Thanks for bringing it up!