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

feature-order lint doesn't check global_attributes #23549

Open
1 of 2 tasks
Elchi3 opened this issue Jun 28, 2024 · 1 comment
Open
1 of 2 tasks

feature-order lint doesn't check global_attributes #23549

Elchi3 opened this issue Jun 28, 2024 · 1 comment
Labels
linter 🏡 Issues or pull requests regarding the tests / linter of the JSON files.

Comments

@Elchi3
Copy link
Member

Elchi3 commented Jun 28, 2024

What type of issue is this?

Linter issue

What is the issue?

The features in https://github.com/mdn/browser-compat-data/blob/main/svg/global_attributes.json are not sorted alphabetically.

What behavior were you expecting?

https://github.com/mdn/browser-compat-data/blob/main/svg/global_attributes.json to be sorted alphabetically (or the linter complaining about it.)

What version(s) of BCD is the issue present in?

  • The current BCD release
  • The current version of the main branch

Do you have anything more you want to share?

Same problem in https://github.com/mdn/browser-compat-data/blob/main/html/global_attributes.json too

@Elchi3 Elchi3 added the linter 🏡 Issues or pull requests regarding the tests / linter of the JSON files. label Jun 28, 2024
@queengooborg
Copy link
Collaborator

It appears that this is caused by the check to see if an object has a __compat property inside of it. This check ensured that we weren't attempting to order the members of compatibility statements, since their ordering was handled separately in the other files. Unfortunately, this check also assumed that the top level of a file had a __compat statement -- in other words, it expected global_attributes to have a __compat statement. This means it's also affecting various files within the JavaScript category, such as grammar and statements.

Ultimately, I think that this is a fatal flaw in the way that the ordering script is programmed. Since we're using the replacer parameter of JSON.stringify, we have no context of what level or path we're at in the JSON; we only know the key and value we're looking at.

To solve this, I believe that a rewrite of the ordering script(s) would be needed. The ordering script would need to be written in a way where we can track the current path and depth, track what kind of object (relative to the schema) we're modifying, and ultimately, ensure we're sorting everything we can. This would also be a good opportunity to merge the ordering scripts into one script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linter 🏡 Issues or pull requests regarding the tests / linter of the JSON files.
Projects
None yet
Development

No branches or pull requests

2 participants