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

Publicly expose reference data #6151

Closed
nex3 opened this issue Jun 15, 2022 · 7 comments · Fixed by #6168
Closed

Publicly expose reference data #6151

nex3 opened this issue Jun 15, 2022 · 7 comments · Fixed by #6168
Labels
status: wip is being worked on by someone type: enhancement a new feature that isn't related to rules

Comments

@nex3
Copy link
Contributor

nex3 commented Jun 15, 2022

What is the problem you're trying to solve?

I'm writing a custom lint that would like to have access to some of the data in lib/reference (specifically shorthandData). Currently that's not publicly available, although it is used by a number of internal lints which suggests that it's useful for implementing reasonable lints.

What solution would you like to see?

Possibly a top-level reference export that exposes the various data tables in question.

@ybiquitous ybiquitous added status: needs discussion triage needs further discussion type: enhancement a new feature that isn't related to rules labels Jun 15, 2022
@ybiquitous
Copy link
Member

@nex3 Thanks for the suggestion. It sounds like a good idea. 👍🏼

I've labeled needs discussion. But I think this idea would be ready if there were no objections or concerns.

@jeddy3
Copy link
Member

jeddy3 commented Jun 20, 2022

@nex3 Thanks for the request. I think we should expose our reference data in our public API to help plugin authors.

Our public API is well-considered, especially our conventions around rules. In contrast, we haven't given much thought to our reference data. For example, keywordSets.js started out as sets of keywords but has become a hodgepodge of other things like units, selectors and properties. We'll want to clean things up before we expose them. Unless someone is motivated to do that in one go, I suggest we:

  • define and agree on some starting conventions
  • align and expose data as it's requested (starting with the shorthand data)

As people request more data, we can clean up and expose the relevant bits. It'll involve digging into the specs to make sure it's named as accurately as it can be.

We could:

  • keep the hierarchy flat, i.e. one level of properties on an exposed .reference property, e.g. stylelint.reference.units
  • prefix for more specific things, e.g. stylelint.reference.lengthUnits
  • use of for nested data, e.g. stylelint.reference.subPropertiesOfLonghandProperties

In that last example, "properties" is the generic, "longhand" the specific and "sub-properties" the nested data (spec ref).

How does that sound?

Lastly, the reference data is mostly comprised of Sets. However, there are some objects and arrays in there. Can we unify what we export?

@ybiquitous
Copy link
Member

@jeddy3 Thank you for clearing considerations! Indeed, it's vital to examine APIs before publishing them. I agree with your suggestions. 👍🏼

@jeddy3 jeddy3 removed the type: enhancement a new feature that isn't related to rules label Jun 21, 2022
@jeddy3
Copy link
Member

jeddy3 commented Jun 21, 2022

@ybiquitous The shorthand data is currently an object. Do you have any thoughts on whether we should continue to use that structure, or is there something else we can use that ties into the Sets of the other reference files?

@ybiquitous
Copy link
Member

@jeddy3 I think it's more helpful to align data structures. Fortunately, shorthandData is used only in the two rules below, so changing the code to fit Set is easy.

  • declaration-block-no-redundant-longhand-properties
  • declaration-block-no-shorthand-property-overrides

@ybiquitous
Copy link
Member

I've created a refactoring PR #6167 before exposing the references.

@ybiquitous
Copy link
Member

@jeddy3 I've tried creating PR #6168 to expose shorthandData following your proposal. Does the PR fit your idea? If so, I'll make the PR ready to review.

@jeddy3 jeddy3 added status: wip is being worked on by someone type: enhancement a new feature that isn't related to rules and removed status: needs discussion triage needs further discussion labels Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone type: enhancement a new feature that isn't related to rules
3 participants