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

FIX: Correctly identify non-ascii JS variables in .vue files #15980

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

FilipRazek
Copy link

Description

Fix issue #13647 by using the correct regex for js variables

Checklist

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.

Try the playground for this PR

@FilipRazek FilipRazek changed the title Fix 13647 FIX: Correctly identify non-ascii JS variables in .vue files Jan 24, 2024
@fisker
Copy link
Member

fisker commented Jan 24, 2024

Do you have a reference for the updated pattern?

@FilipRazek
Copy link
Author

From this SO thread (also mentioned in the constants.js file). However, the pattern is quite long - and probably not very maintainable, but I don't have enough experience with parsing to know what would be sufficient.

@FilipRazek
Copy link
Author

The tests pass with a much simpler regex - [^\s;,]+, matching any character that is not whitespace and not a semicolon/colon.

@fisker
Copy link
Member

fisker commented Jan 26, 2024

@FilipRazek
Copy link
Author

FilipRazek commented Feb 22, 2024

Alright, thanks for your comment! The logic seems quite complex and I'm not sure I'm able to understand everything. However, from what I've seen, the vModel calls a isSimpleIdentifier function, defined as such:

const nonIdentifierRE = /^\d|[^\$\w]/
export const isSimpleIdentifier = (name: string): boolean =>
  !nonIdentifierRE.test(name)

If there is more logic to implement, could you please tell me where to look?
Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants