Add shorthand properties reference to public API #6168

ybiquitous · 2022-06-23T00:07:21Z

Which issue, if any, is this issue related to?

Is there anything in the PR that needs further explanation?

This pull request is following the suggestion below by @jeddy3 in #6151 (comment):

align and expose data as it's requested (starting with the shorthand data)

The type definition is generated by the following script:

const shorthandData = require("./lib/reference/shorthandData")
console.log('export type ShorthandData = {')
for (const key of Object.keys(shorthandData).sort()) {
	console.log(`\t'${key}': Set<string>;`)
}
console.log('};')

The type definition is generated by the following script: ```js const shorthandData = require("./lib/reference/shorthandData") console.log('export type ShorthandData = {') for (const key of Object.keys(shorthandData)) { console.log(`\t'${key}': Set<string>;`) } console.log('};') ```

ybiquitous · 2022-06-23T00:28:39Z

[ask] The shorthandData name seems unclear to me. Is it acceptable, or is there a better name?

jeddy3 · 2022-06-23T12:32:06Z

@ybiquitous Thank you. It's looking good.

[ask] The shorthandData name seems unclear to me. Is it acceptable, or is there a better name?

Let's go with subPropertiesOfLonghandProperties as per #6151 (comment).

Future property sets could include acceptCustomIdentsProperties and these:

stylelint/lib/reference/keywordSets.js

Lines 411 to 423 in bd8c7fd

    
           keywordSets.shorthandTimeProperties = new Set(['transition', 'animation']); 
        
           keywordSets.longhandTimeProperties = new Set([ 
        
           	'transition-duration', 
        
           	'transition-delay', 
        
           	'animation-duration', 
        
           	'animation-delay', 
        
           ]); 
        
           keywordSets.timeProperties = uniteSets( 
        
           	keywordSets.shorthandTimeProperties, 
        
           	keywordSets.longhandTimeProperties, 
        
           );

(Which are in the keywordSets.js at the moment...)

What file structure shall we use? Choices are:

1 file per set, e.g. reference/subPropertiesOfLonghandProperties.js
1 file per group, e.g. reference/properties.js (which would contain all the above)
a single file for all the sets, e.g. reference/reference.js

I feel like either of the last two will be easiest to work with and maintain. For the 1 file per group, offhand I can think of the following groups we have:

properties
functions
units
keywords
selectors (which would include all the HTML elements but renamed as type selectors)
media features
operators
combinators

ybiquitous · 2022-06-23T14:01:58Z

@jeddy3 Thanks for your helpful feedback! The idea of "1 file per group" seems so nice. 👍🏼

I'll push commits into this pull request, following the ideas of subPropertiesOfLonghandProperties and "1 file per group".

jeddy3 · 2022-06-23T16:35:30Z

I'll push commits into this pull request, following the ideas of subPropertiesOfLonghandProperties and "1 file per group".

Oops, I got it back-to-front. From the spec:

Some properties are shorthand properties, meaning that they allow authors to specify the values of several properties with a single property. A shorthand property sets all of its longhand sub-properties, exactly as if expanded in place.

Let's go with longhandSubPropertiesOfShorthandProperties to be accurate with the wording in the spec, even if it's a bit verbose.

ybiquitous · 2022-06-25T00:24:23Z

I have one more suggestion: what if using Map instead of Object?

This MDN doc describes the differences between Map and Object well. I think Map could be safer and more stable than Object, especially as a public API, even if Map doesn't support JSON serialization natively. E.g.

// Using Object
const longhandSubPropertiesOfShorthandProperties = {
  margin: new Set(['margin-top', 'margin-bottom', 'margin-left', 'margin-right']),
  // ...
};
longhandSubPropertiesOfShorthandProperties.margin; //=> Set(...)

// Using Map
const longhandSubPropertiesOfShorthandProperties = new Map([
  ['margin', new Set(['margin-top', 'margin-bottom', 'margin-left', 'margin-right'])],
  // ...
]);
longhandSubPropertiesOfShorthandProperties.get('margin'); //=> Set(...)

What do you think?

ybiquitous · 2022-06-25T00:58:19Z

lib/reference/properties.js

-	padding: new Set(['padding-top', 'padding-bottom', 'padding-left', 'padding-right']),
+/** @type {import('stylelint').LonghandSubPropertiesOfShorthandProperties} */
+const longhandSubPropertiesOfShorthandProperties = {
+	/* eslint sort-keys: 'error' */


[note] I've sorted properties alphabetically for findability (the diff increases, though).

ybiquitous · 2022-06-25T00:59:13Z

lib/reference/properties.js

@@ -91,6 +47,18 @@ module.exports = {
 		'border-block-start-style',
 		'border-block-start-color',
 	]),
+	'border-bottom': new Set([
+		// prettier-ignore


[note] To align always multi-line for readability.

jeddy3 · 2022-06-25T11:31:32Z

I have one more suggestion: what if using Map instead of Object?

Sounds good to me.

All the other changes are looking great, by the way!

ybiquitous · 2022-06-27T01:20:46Z

types/stylelint/index.d.ts

+			| 'text-decoration'
+			| 'text-emphasis'
+			| 'transition',
+			ReadonlySet<string>


[note] ReadonlyMap and ReadonlySet are built-in types.

ybiquitous · 2022-06-27T01:21:48Z

lib/rules/declaration-block-no-shorthand-property-overrides/index.js


 					return;
 				}

 				for (const longhandProp of overrideables) {
-					if (!Object.prototype.hasOwnProperty.call(declarations, prefix + longhandProp)) {
+					const declaration = declarations.get(prefix + longhandProp);


[note] Refactor to use Map instead of Object; hasOwnProperty is no longer needed (safe access).

Much nicer.

ybiquitous · 2022-06-27T01:23:16Z

lib/rules/declaration-block-no-redundant-longhand-properties/index.js

+				shorthands.push(shorthand);
+				longhandToShorthands.set(longhand, shorthands);
+			}
+		}


[note] Refactor to use Map and for...of instead of Object and reduce() for readability.

ybiquitous · 2022-06-27T01:26:14Z

@jeddy3 I've switched from Object to Map and Set (ReadonlyMap and ReadonlySet in TypeScript), and also refactored declaration-block-* rules using Map.

jeddy3

Fantastic, thank you!

I think we have a good direction here for exposing our reference data.

jeddy3 · 2022-06-27T10:21:09Z

Added: longhand sub-properties of shorthand properties reference data to public API (#6168).

ybiquitous · 2022-06-27T14:19:45Z

@jeddy3 Thank you for all of your reviews. 😊 👍🏼

ybiquitous force-pushed the expose-shorthand-props-reference branch from 37fa87e to d12beec Compare June 23, 2022 00:23

Sort property names in type definition alphabetically

7074058

ybiquitous mentioned this pull request Jun 23, 2022

Publicly expose reference data #6151

Closed

jeddy3 changed the title ~~Expose shorthand properties reference as public API~~ Add shorthand properties reference to public API Jun 23, 2022

nex3 mentioned this pull request Jun 24, 2022

Publicly expose reference data #6152

Closed

ybiquitous added 2 commits June 25, 2022 09:37

Rename shorthandData.js and shorthandData

166e0e4

Merge branch 'main' into expose-shorthand-props-reference

394a760

ybiquitous marked this pull request as ready for review June 25, 2022 00:38

ybiquitous marked this pull request as draft June 25, 2022 00:42

Refactor for readability

b6c2e38

ybiquitous commented Jun 25, 2022

View reviewed changes

ybiquitous marked this pull request as ready for review June 25, 2022 00:59

ybiquitous added 2 commits June 27, 2022 10:10

Use Map instead of Object

e6adad8

Use ReadonlySet

1e374a3

ybiquitous commented Jun 27, 2022

View reviewed changes

ybiquitous requested a review from jeddy3 June 27, 2022 01:24

jeddy3 approved these changes Jun 27, 2022

View reviewed changes

jeddy3 merged commit 6ed812b into main Jun 27, 2022

jeddy3 deleted the expose-shorthand-props-reference branch June 27, 2022 10:19

jeddy3 mentioned this pull request Jun 27, 2022

Refactor reference data into groups #6179

Merged

sync-by-unito bot mentioned this pull request Sep 6, 2022

build(deps-dev): bump stylelint and stylelint-config-standard numbersprotocol/capture-lite#2033

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add shorthand properties reference to public API #6168

Add shorthand properties reference to public API #6168

ybiquitous commented Jun 23, 2022 •

edited by jeddy3

ybiquitous commented Jun 23, 2022

jeddy3 commented Jun 23, 2022 •

edited

ybiquitous commented Jun 23, 2022

jeddy3 commented Jun 23, 2022

ybiquitous commented Jun 25, 2022

ybiquitous Jun 25, 2022

ybiquitous Jun 25, 2022

jeddy3 commented Jun 25, 2022

ybiquitous Jun 27, 2022

ybiquitous Jun 27, 2022

jeddy3 Jun 27, 2022

ybiquitous Jun 27, 2022

ybiquitous commented Jun 27, 2022

jeddy3 left a comment

jeddy3 commented Jun 27, 2022

ybiquitous commented Jun 27, 2022

Add shorthand properties reference to public API #6168

Add shorthand properties reference to public API #6168

Conversation

ybiquitous commented Jun 23, 2022 • edited by jeddy3

ybiquitous commented Jun 23, 2022

jeddy3 commented Jun 23, 2022 • edited

ybiquitous commented Jun 23, 2022

jeddy3 commented Jun 23, 2022

ybiquitous commented Jun 25, 2022

ybiquitous Jun 25, 2022

Choose a reason for hiding this comment

ybiquitous Jun 25, 2022

Choose a reason for hiding this comment

jeddy3 commented Jun 25, 2022

ybiquitous Jun 27, 2022

Choose a reason for hiding this comment

ybiquitous Jun 27, 2022

Choose a reason for hiding this comment

jeddy3 Jun 27, 2022

Choose a reason for hiding this comment

ybiquitous Jun 27, 2022

Choose a reason for hiding this comment

ybiquitous commented Jun 27, 2022

jeddy3 left a comment

Choose a reason for hiding this comment

jeddy3 commented Jun 27, 2022

ybiquitous commented Jun 27, 2022

ybiquitous commented Jun 23, 2022 •

edited by jeddy3

jeddy3 commented Jun 23, 2022 •

edited