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

Refactor to improve types for some rules #5498

Merged
merged 1 commit into from Aug 25, 2021
Merged

Conversation

ybiquitous
Copy link
Member

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

Part of #4496

Is there anything in the PR that needs further explanation?

This change removes // @ts-nocheck from the following rules:

  • keyframe-declaration-no-important
  • keyframes-name-pattern
  • length-zero-no-unit
  • linebreaks

This change removes `// @ts-nocheck` from the following rules:
- `keyframe-declaration-no-important`
- `keyframes-name-pattern`
- `length-zero-no-unit`
- `linebreaks`
function reportNewlineError(line, column) {
// Creating a node manually helps us to point to empty lines.
const node = postcss.rule({
source: {
start: { line, column },
start: { line, column, offset: 0 },
input: new postcss.Input(''),
Copy link
Member Author

Choose a reason for hiding this comment

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

[note]

...but, I'm not sure if the added dummy values are appropriate. 🤔

Copy link
Member

Choose a reason for hiding this comment

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

I think it's acceptable. We can always revisit.


if (context.fix) {
const propertiesToUpdate = ['selector', 'value', 'text'];
const rawsPropertiesToUpdate = ['before', 'after'];
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] This is a more type-safe way, so that the property sets are unnecessary.

Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

Fantastic, thanks!

The @param above functions adds so much clarity, especially when destructuring is being used, e.g.:

/**
 * @param {import('postcss').Declaration} decl
 * @param {import('postcss-value-parser').Node[]} nodes
 * @param {number} index
 */
function isLineHeightValue({ prop }, nodes, index) { .. }

@jeddy3 jeddy3 merged commit 5d98cc3 into v14 Aug 25, 2021
@jeddy3 jeddy3 deleted the refactor-types-for-some-rules branch August 25, 2021 13:19
@ybiquitous
Copy link
Member Author

@jeddy3 I always appreciate your review! The progress is almost 50%: 😃

$ ls -l lib/rules | wc -l
     201

$ git grep -l '@ts-nocheck' -- lib | wc -l
     109

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants