-
Notifications
You must be signed in to change notification settings - Fork 5
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
Exclude JSON identifiers from parsing #18
base: master
Are you sure you want to change the base?
Conversation
index.js
Outdated
const { value } = node; | ||
const { value, parent } = node; | ||
const nodeIsIdentifier = parent.type === 'JSONProperty' && parent.key === node | ||
if (nodeIsIdentifier) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to exclude the JavaScript property name from the check as well, you'll need to make the same changes to the JavaScript check method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the help @ota-meshi! @constgen if you'd like to include that check in the PR, let me know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I realized. These two JS expressions produce different AST nodes:
{ name: 'value' }
-name
is Identifier type{ 'name': 'value' }
-'name'
is Literal type
So I am going to add additional checks for JS too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excluded property names from parsing and added a test for this. But tests are not run automatically on any CI/CD
Thank you @constgen for making this PR. I think I might later on refactor this so that I can use this in conjunction with |
|
Hey @constgen, |
I am going to do this in this PR. Just give me your opinion on the description of the options names and default values |
Hey @constgen , |
We don't need to analyze strings that play a role of property names