microsoft / fluentui

Fluent UI web represents a collection of utilities, React components, and web components for building web applications.
https://react.fluentui.dev
Other
18.26k stars 2.71k forks source link

[Bug]: Cannot commit change to `/tools` #31751

Open spmonahan opened 2 months ago

spmonahan commented 2 months ago

Library

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

System Info

System:
    OS: Linux 5.15 Ubuntu 20.04.6 LTS (Focal Fossa)
    CPU: (8) x64 Intel(R) Core(TM) i7-1065G7 CPU @ 1.30GHz
    Memory: 9.95 GB / 15.42 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash
  Browsers:
    Chrome: 125.0.6422.60

Are you reporting Accessibility issue?

no

Reproduction

Make a change in /tools, run git commit

Bug Description

Actual Behavior

Linting during git commit fails when changing a file in /tools.

Make a (valid) change to a file in /tools: image

Save then run git commit

image

Expected Behavior

Valid changes should not fail linting during commit. This can be worked around by using the --no-verify flag on git commit.

Logs

No response

Requested priority

Normal

Products/sites affected

No response

Are you willing to submit a PR to fix?

no

Validations

Hotell commented 2 months ago

thank you for report ! this is indeed a bug/limitation of our linting setup.

This is failing because workspace-plugin uses type aware rules, which are turned off during commit checks that run lint-staged.

ATM our only mechanism is to imperatively register type-aware rules within eslint-plugin or per package config ( which implies using javascript for eslint configuration which we wanna avoid ) https://github.com/microsoft/fluentui/blob/5a20835e6e952c80aa18c83531db494dd85b2def/packages/eslint-plugin/src/utils/configHelpers.js#L178-L179

How to Fix this:

  1. Quick fix is to migrate tools eslint to .js

  2. Long term fix:

    • implement better approach how to determine what rules need typescript program and disable them when necessary in declarative way
    • move lint checks to pre-push instead pre-comit