Skip to content

Custom ESLint rule to disable sorting for ref attribute, as it needs to be the last property in JSX to ensure it is up-to-date. #7659

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

Closed
1 of 4 tasks
Elijbet opened this issue Sep 1, 2023 · 2 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. eslint-plugin-calcite-components Issues specific to the @esri/eslint-plugin-calcite-components package. estimate - 5 A few days of work, definitely requires updates to tests. p - medium Issue is non core or affecting less that 60% of people using the library refactor Issues tied to code that needs to be significantly reworked.

Comments

@Elijbet
Copy link
Contributor

Elijbet commented Sep 1, 2023

Description

  • This GitHub issue relates to an existing bug in the Stencil library (bug: ref called in specified order and not after initializing element with all its attributes/properties stenciljs/core#4074). The current behavior is that the order of ref usage can result in outdated props/attributes when the callback is invoked.

  • The expected behavior is for ref to be called immediately after the element is fully initialized with all props/attributes, aligning with similar mechanisms in other libraries like React and Preact. Properties are applied in the order they are specified. If you place the ref at the end of the component's properties, it ensures that the component's render and updates are processed before the ref callback is triggered. This means the ref callback will have access to the fully rendered and updated component or DOM element.

  • The suggested solution involves setting ref as the last property in JSX to ensure that node attributes/properties are up-to-date. However, this requires disabling the jsx-sort-props rule specifically for this case using an ESLint comment (eslint-disable-next-line) (refactor: move ref prop last to ensure ref node is in sync #6530, chore: add explainers to disabled jsx-sort-props on ref prop #7584).

  • The request is to create a custom ESLint rule that catches this pattern instead of relying on manual comments throughout the repository.

Proposed Advantages

Provides an automated solution to manual comments to disable sorting rules throughout the repository.

Which Component

All.

Relevant Info

No response

Calcite package

  • @esri/calcite-components
  • @esri/calcite-components-react
  • @esri/calcite-design-tokens
  • @esri/eslint-plugin-calcite-components
@Elijbet Elijbet added refactor Issues tied to code that needs to be significantly reworked. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Sep 1, 2023
@github-actions github-actions bot added the calcite-components Issues specific to the @esri/calcite-components package. label Sep 1, 2023
@Elijbet Elijbet self-assigned this Sep 1, 2023
@Elijbet Elijbet added eslint-plugin-calcite-components Issues specific to the @esri/eslint-plugin-calcite-components package. and removed calcite-components Issues specific to the @esri/calcite-components package. labels Sep 1, 2023
@Elijbet Elijbet added 2 - in development Issues that are actively being worked on. and removed 0 - new New issues that need assignment. labels Sep 3, 2023
Elijbet added a commit that referenced this issue Nov 2, 2023
…eep it as last property in JSX to ensure it is up-to-date (#7669)

**Related Issue:** #7659

## Summary
Custom ESLint rule to provide a more granular prop ordering mechanism by
forcing the `ref` attribute to be kept as the last property so we can
ensure it is up-to-date.

Added a test file for the new custom rules: the file references
different use cases, which are 1 good and 1 bad usage files.
jcfranco added a commit that referenced this issue Nov 8, 2023
**Related Issue:** #7669, #7659

## Summary

Refactor to improve on `enforce-ref-last-prop` ESLint rule docs.

---------

Co-authored-by: JC Franco <[email protected]>
jcfranco added a commit that referenced this issue Nov 9, 2023
**Related Issue:** #7669, #7659

## Summary

Refactor to improve on `enforce-ref-last-prop` ESLint rule docs.

---------

Co-authored-by: JC Franco <[email protected]>
@jcfranco jcfranco added this to the 2023 November Priorities milestone Jan 27, 2024
@jcfranco jcfranco added estimate - 5 A few days of work, definitely requires updates to tests. p - medium Issue is non core or affecting less that 60% of people using the library 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed needs triage Planning workflow - pending design/dev review. 2 - in development Issues that are actively being worked on. labels Jan 27, 2024
Copy link
Contributor

Installed and assigned for verification.

@github-actions github-actions bot assigned geospatialem and DitwanP and unassigned Elijbet Jan 27, 2024
@jcfranco
Copy link
Member

Updated labels and verified locally. 🎉

The next step for this rule will be tackled in #8080.

@jcfranco jcfranco added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. eslint-plugin-calcite-components Issues specific to the @esri/eslint-plugin-calcite-components package. estimate - 5 A few days of work, definitely requires updates to tests. p - medium Issue is non core or affecting less that 60% of people using the library refactor Issues tied to code that needs to be significantly reworked.
Projects
None yet
Development

No branches or pull requests

4 participants