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

eslint error with isConformant.ts in some packages #20720

Closed
ling1726 opened this issue Nov 23, 2021 · 6 comments
Closed

eslint error with isConformant.ts in some packages #20720

ling1726 opened this issue Nov 23, 2021 · 6 comments
Assignees

Comments

@ling1726
Copy link
Member

Seen in #20692

To repro simply run yarn eslint . in react-portal.

image

It seems that none of the tsconfigs actually include src/common/isConformant.ts however, it has currently worked or other packages both in that PR or already completed ones like: #20599 #20673 #20675

@Hotell
Copy link
Contributor

Hotell commented Nov 23, 2021

I played with it few minutes, here is a quickfix -> isConformant is actually not used at all, that's what makes it fail. if you import it in test file then tsconfig.lib.json will "know" about it although it's being excluded in exclude, thus eslint wont complain.

2021-11-23 at 17 22

💡 So to unblock your PR either remove that file completely or use it within test.

In general I think the linting approach will need to be redone because project references are not supported by typescript-eslint, but for some reason our lint configuration "works" as is for now

@Hotell
Copy link
Contributor

Hotell commented Nov 23, 2021

additional observation:

we invoke eslint via just differently than how IDE/native eslint invokes it

  • we execute it only against src folder
  • fluentui/packages/react-portal/src --ext ".ts,.tsx,.js,.jsx" --config "/microsoft/fluentui/packages/react-portal/.eslintrc.json" --color

This is "problematic" as IDE is giving use squigless where CI wont ( bad DX )

@ecraig12345
Copy link
Member

I think the intent with the just version was to expand it to include config/other files outside of src as well, but nobody had time to add that.

@Hotell
Copy link
Contributor

Hotell commented Dec 3, 2021

I think the intent with the just version was to expand it to include config/other files outside of src as well, but nobody had time to add that.

can we transform this to some knowledge base - actionable item ?

@msft-fluent-ui-bot msft-fluent-ui-bot added the Resolution: Soft Close Soft closing inactive issues over a certain period label Jun 1, 2022
@msft-fluent-ui-bot
Copy link
Collaborator

Because this issue has not had activity for over 180 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.

@msft-fluent-ui-bot
Copy link
Collaborator

Because this reported issue has not had any activity for over 180 days, we're automatically closing it for house-keeping reasons.

Still require assistance? Please, create a new issue with up-to date details and latest version of Fluent.

@microsoft microsoft locked as resolved and limited conversation to collaborators Jan 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants