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.

Sign up for GitHub

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

Jump to bottom

Add dashed-ident-no-missing-reference-function #5044

Closed
wants to merge 21 commits into from
Closed

Add dashed-ident-no-missing-reference-function #5044

wants to merge 21 commits into from

Conversation

jeddy3
Copy link
Member

@jeddy3 jeddy3 commented Nov 18, 2020 •
edited

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

Closes #5000

Is there anything in the PR that needs further explanation?

I wanted to put together something to help feed into the discussion in #5000.

I've dropped declaration-value from the name and went with dashed-ident-no-missing-reference-function as I now feel declaration-value is redundant because dashed-idents outside of declarations values don't use reference functions.

In the future, we can add ignoreDashedIdents and ignoresFunction options:

"rules": {
  "dashed-ident-no-missing-reference-function": [true, {
    "ignoreDashedIdents": ["--my-js-ident"],
    "ignoreFunctions": ["--custom"]
  }]
}

Which would allow:

a {
  prop: --my-js-ident;
  prop: --custom(--foo);
}

Like the font-family-no-missing-generic-family-keyword rule, this rule sits in the possible errors category.

mattxwang
mattxwang approved these changes Nov 21, 2020
View reviewed changes
Copy link
Member

@mattxwang mattxwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking through the linked discussion, test cases, and implementation, this seems good to me!

P.S. a { color: calc(var(--foo) + --bar)); } is a great test case for this to work on!

lib/rules/dashed-ident-no-missing-reference-function/index.js
`Unexpected missing dashed-ident reference function for "${dashedIdent}"`,
});

const REFERENCE_FUNCTIONS = ['color', 'env', 'var'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious - are there many other uses cases for this set of reference functions? Can't think of any in my normal use-cases, but thinking about if we should pull this out to the constants files.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can pull them out into a reference file if they end up being shared by another rule.

@jeddy3
Copy link
Member Author

jeddy3 commented Nov 21, 2020

Added a small commit to use Set to align with the changes in #5048.

ybiquitous
ybiquitous approved these changes Dec 10, 2020
View reviewed changes
Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found one typo, but this change looks good to me! 👍

lib/rules/dashed-ident-no-missing-reference-function/README.md Show resolved Hide resolved
@jeddy3 jeddy3 mentioned this pull request Jan 11, 2021
Closed
6 tasks
jeddy3 and others added 20 commits May 14, 2021 07:15
@jeddy3
Remove deprecated rules (#5295)
8c48068
@jeddy3
Update CHANGELOG.md
2f01778
@jeddy3
Remove function-calc-no-invalid (#5296)
4b82c24 
* Remove function-calc-no-invalid

* Remove calc parser

* Remove ignoring of calc parser
@jeddy3
Update CHANGELOG.md
0b3fa84
@ybiquitous
Drop Node 10 support for ESM (#5303)
37e1190 
This change is a part of the next major version (v14).

- Remove Node 10 from CI
- Update `engines.node`

In addition, this removes needless `CI: true` (just a refactoring).
See <https://github.blog/changelog/2020-04-15-github-actions-sets-the-ci-environment-variable-to-true/>
@ybiquitous
Update CHANGELOG.md
90a174d
@ybiquitous
Refactor to remove Number.isInteger polyfill (#5302)
e4b6141 
We will no longer need the polyfill on the next major release (v14).
@ybiquitous
Add lang to code blocks in Markdown (#5299)
887f03e 
* Add lang to code blocks in Markdown

This change aims to make documents more readable and avoid syntax errors
by adding a language specifier to code blocks in Markdown.

To perform this task efficiently, I use two remark plugins:
- [remark-lint-code-block-syntax](https://github.com/ybiquitous/remark-lint-code-block-syntax)
- [remark-lint-fenced-code-flag](https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-fenced-code-flag)

```diff
--- a/package.json
+++ b/package.json
@@ -66,7 +66,9 @@
   },
   "remarkConfig": {
     "plugins": [
-      "@stylelint/remark-preset"
+      "@stylelint/remark-preset",
+      "remark-lint-code-block-syntax",
+      "remark-lint-fenced-code-flag"
     ]
   },
   "jest": {
@@ -189,6 +191,8 @@
     "postcss-import": "^12.0.1",
     "prettier": "^2.2.1",
     "remark-cli": "^9.0.0",
+    "remark-lint-code-block-syntax": "^0.2.1",
+    "remark-lint-fenced-code-flag": "^2.0.1",
     "typescript": "^4.2.4"
   },
   "engines": {
```

Note: I do not recommend the addition of the plugins because they produce false positives.

* Remove deprecated files

* Remove deprecated files again
@jeddy3
Remove syntax option (#5297)
52f99c6
@jeddy3
Update CHANGELOG.md
1e4ca41
@qubyte
Refactor async calls to fs to use fs/promises (#5271) (#5306)
e0004aa
@ybiquitous
Refactor lib/__tests__/standalone-cache.test.js (#5311)
2345a21 
* Refactor `lib/__tests__/standalone-cache.test.js`

- Reduce redundant callbacks via `async/await` syntax. (#4881)
- Use `fs.existsSync()` instead of `fs.access()`.
  See <https://nodejs.org/api/fs.html#fs_fs_existssync_path>
- Fix the disabled test. (#5309)
- Inline redundant local variables.
- Make expectations using `typeof` more accurate.

* Reduce local variables by improving `getConfig()`
@doing-art
Add autofix to function-calc-no-unspaced-operator (#5273)
3d5725e
@jeddy3
Update CHANGELOG.md
a3460ef
@jeddy3
Fix false positives for flex in length-zero-no-unit (#5315)
52a67ab
@jeddy3
Update CHANGELOG.md
9152ab4
@jeddy3
Add ignoreFunctions: [] to length-zero-no-unit (#5314)
492ae33
@jeddy3
Update CHANGELOG.md
697fc15
@jeddy3
Document what each formatter does (#5313)
d540d78
@ybiquitous
Refactor to remove autoprefixer dependency (#5312)
6086255
@alexander-akait
Copy link
Member

Found this name is very misleading, in css we have env https://developer.mozilla.org/en-US/docs/Web/CSS/env() and in future them will be more

@jeddy3 jeddy3 mentioned this pull request May 19, 2021
Merged
@jeddy3
Copy link
Member Author

jeddy3 commented May 19, 2021

Found this name is very misleading, in css we have env https://developer.mozilla.org/en-US/docs/Web/CSS/env() and in future them will be more

Yes, I had a similar thought.

Opened #5317.

@jeddy3 jeddy3 closed this May 19, 2021
@jeddy3 jeddy3 deleted the add-dashed-ident branch May 19, 2021 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ybiquitous ybiquitous ybiquitous approved these changes

@mattxwang mattxwang mattxwang approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add dashed-ident-no-missing-reference-function
6 participants
@jeddy3 @alexander-akait @ybiquitous @mattxwang @qubyte @doing-art