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

[Feature]: New lint rule to test event callback type and deprecate/migrate conformance test consistent-callback-args #30372

Closed
1 task done
YuanboXue-Amber opened this issue Jan 23, 2024 · 3 comments
Assignees
Labels
Fluent UI react-components (v9) Resolution: Soft Close Soft closing inactive issues over a certain period Type: Feature

Comments

@YuanboXue-Amber
Copy link
Contributor

YuanboXue-Amber commented Jan 23, 2024

Library

React Components / v9 (@fluentui/react-components)

Describe the feature that you would like added

This is an issue to gather all necessary steps to add testing for new event type.

A new type for event callback is proposed in - https://github.com/microsoft/fluentui/blob/master/rfcs/react-components/convergence/event-handlers-event-type.md

We need to:

  1. deprecate existing consistent-callback-args test feat(react-conformance): Deprecate conformance test consistent-callback-args #30301
  2. add lint rule for new type feat(eslint-rules): add consistent-callback-type rule for event callback typing for react-components and enable it in all v9 packages #30293
  3. update existing consistent-callback-args test to make sure it doesn't run for new callbacks added to existing component BREAKING(react-conformance): update consistent-callback-args option to run for existing callbacks only #30376

Have you discussed this feature with our team

Yes

Additional context

No response

Validations

  • Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

Priority

High

@Hotell
Copy link
Contributor

Hotell commented Jan 24, 2024

  1. can we improve the issue title? it's bit confusing IMO what's the feature request

  2. regarding consistent-callback-args conformance test:

copy paste from PR for better visibility (#30376 (review))

This rule is the last rule that needs to create TS program when running these conformance test which is extremely slow #27359.

The end goal of conformance tests is to have all of those rules ported to ESlint.

Aim of this issues should be:

  • migrate logic of this conformance test to our workspace lint rules
  • remove this rule from conformance package

this has 2 major benefits:

  • performance of tests
  • improved DX in comparison with current approach. lint rule can be then enabled directly in source code for problematic callbacks, for fine grained control and getting rid of unnecessary decoupling of concerns

@YuanboXue-Amber YuanboXue-Amber changed the title [Feature]: Test for new event type [Feature]: New lint rule to test event callback type and deprecate/migrate conformance test consistent-callback-args Jan 25, 2024
@YuanboXue-Amber
Copy link
Contributor Author

YuanboXue-Amber commented Jan 25, 2024

@Hotell on your comment for migrating consistent-callback-args, the last rule that creates TSProgram:
to migrate it into a lint rule, the lint rule should be active for only certain callbacks in a few files. I don't know a clean way to achieve that.

Update:
Also a bigger problem - this test builds TS program and utilize typescript typeChecker, migrating it can kill lint performance instead of unit test performance :/

This issue has not had activity for over 180 days! We're adding Soft close label and will close it soon for house-keeping purposes.
Still require assistance? Please add comment - "keep open".

@microsoft-github-policy-service microsoft-github-policy-service bot added the Resolution: Soft Close Soft closing inactive issues over a certain period label Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fluent UI react-components (v9) Resolution: Soft Close Soft closing inactive issues over a certain period Type: Feature
Projects
None yet
Development

No branches or pull requests

2 participants