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

Enhance no-direct-date ESLint rule #9032

Closed
6 tasks done
aaemnnosttv opened this issue Jul 17, 2024 · 2 comments
Closed
6 tasks done

Enhance no-direct-date ESLint rule #9032

aaemnnosttv opened this issue Jul 17, 2024 · 2 comments
Labels
P1 Medium priority Team S Issues for Squad 1 Type: Infrastructure Engineering infrastructure & tooling

Comments

@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented Jul 17, 2024

Feature Description

In #8483 we introduced a new ESLint rule to avoid the use of Date directly, to encourage developers to use the reference date where relevant, but also to use our various date utility functions avoid introducing unexpected behavior related to the quirks of using Date directly.

In the initial implementation, the rule was made specific to a new Date with no arguments, however this misses instances which could be created using our date string format YYYY-MM-DD which will be misinterpreted based on the current timezone.

eg.
image


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • The no-direct-date rule should be updated to flag all uses of new Date with less than 3 arguments (or if problematic, any number of arguments)
  • Existing flagged usage of new Date( arg [, arg1...] ) (i.e. 1 or more args) should be audited and either:
    • Ignored with a comment explaining why they are not using the reference date or SK date utility
    • Updated to use the reference date or relevant date utility where appropriate
    • The report message should also be updated to suggest the use of our date utility functions (not only the reference date)
  • The ignoreFiles configuration in the eslint plugin config for the rule should be moved to the .eslintrc.json

Implementation Brief

  • Update packages/eslint-plugin/rules/no-direct-date.js:

    • To catch new Date( arg [, arg1...] ), update NewExpression callback to report a rule violation if the node.callee is Date and the node.arguments.length is > 0 and < 3.
    • To catch Date( arg [, arg1...] ), update CallExpression callback to report a rule violation if the node.callee is Date and the node.arguments.length is > 0 and < 3.
  • Move the ignoreFiles configuration in the eslint plugin to the overrides block of the .eslintrc.json file.

  • Update all new lint errors to use the plugin reference date or ingnore the rule.

Test Coverage

  • Update packages/eslint-plugin/rules/no-direct-date.test.js, adding invalid usages of new Date( arg [, arg1...] ) and Date( arg [, arg1...] ) with one or two args and a valid case with three or more arguments.

QA Brief

  • No QA necessary.

Changelog entry

  • N/A
@aaemnnosttv aaemnnosttv added P1 Medium priority Type: Infrastructure Engineering infrastructure & tooling labels Jul 17, 2024
@benbowler benbowler assigned benbowler and unassigned benbowler Sep 3, 2024
@tofumatt tofumatt self-assigned this Sep 9, 2024
@tofumatt
Copy link
Collaborator

tofumatt commented Sep 9, 2024

IB ✅

@tofumatt tofumatt removed their assignment Sep 9, 2024
@binnieshah binnieshah added the Team S Issues for Squad 1 label Sep 18, 2024
@tofumatt tofumatt self-assigned this Oct 3, 2024
@tofumatt tofumatt removed their assignment Oct 3, 2024
@tofumatt tofumatt assigned aaemnnosttv and unassigned tofumatt Oct 9, 2024
@aaemnnosttv
Copy link
Collaborator Author

No QA needed here; sending to Approval 👍

@aaemnnosttv aaemnnosttv removed their assignment Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Medium priority Team S Issues for Squad 1 Type: Infrastructure Engineering infrastructure & tooling
Projects
None yet
Development

No branches or pull requests

4 participants