Skip to content

fix: eslint crashing#10491

Merged
emrysal merged 2 commits into
calcom:mainfrom
O4epegb:fix-type-for-eslint-parser
Aug 1, 2023
Merged

fix: eslint crashing#10491
emrysal merged 2 commits into
calcom:mainfrom
O4epegb:fix-type-for-eslint-parser

Conversation

@O4epegb

@O4epegb O4epegb commented Jul 31, 2023

Copy link
Copy Markdown
Contributor

What does this PR do?

yarn lint crashes for me locally (and on CI too apparently, e.g. other random PR https://github.com/calcom/cal.com/actions/runs/5718689486/job/15495018755?pr=10488#step:5:18) because in App.tsx one of the type has the same name as App component, both App, basically shadowing
It's actually valid TS code, but it seems it requires some package updates to parse without errors, and since right now it's hard to update package versions and yarn.lock correctly, I decided to just fix this one line.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

Run yarn lint, it should not crash

Mandatory Tasks

  • Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

@vercel

vercel Bot commented Jul 31, 2023

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 1, 2023 3:39pm

@vercel

vercel Bot commented Jul 31, 2023

Copy link
Copy Markdown

@O4epegb is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

github-actions Bot commented Jul 31, 2023

Copy link
Copy Markdown
Contributor

Thank you for following the naming conventions! 🙏

import type { RouterOutputs } from "@calcom/trpc/react";
import { trpc } from "@calcom/trpc/react";
import type { App as AppType } from "@calcom/types/App";
import type { App as AppType, AppFrontendPayload } from "@calcom/types/App";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only change basically, everything else is autoformat etc

@O4epegb O4epegb changed the title fix: type name clashing for eslint parser fix: eslint crashing Jul 31, 2023
@github-actions

github-actions Bot commented Jul 31, 2023

Copy link
Copy Markdown
Contributor

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@O4epegb

O4epegb commented Jul 31, 2023

Copy link
Copy Markdown
Contributor Author

And now after the fix there are 55 eslint-prettier errors, should I fix them here too? Or make them warnings?

@nicktrn

nicktrn commented Jul 31, 2023

Copy link
Copy Markdown
Contributor

Can't help here but came to say well done! Saw that error before myself but never got the full call stack, so decided to ignore it in the end. Others have had the problem too, see #10292.

emrysal
emrysal previously approved these changes Aug 1, 2023

@emrysal emrysal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing fix! Turns out our lint:report command has been failing silently and our lint analyse actually succeeding, so the errors that you found made it in over the last week.

lgtm

@emrysal emrysal enabled auto-merge (squash) August 1, 2023 15:22
@emrysal emrysal merged commit 7c64b6a into calcom:main Aug 1, 2023
@O4epegb O4epegb deleted the fix-type-for-eslint-parser branch August 1, 2023 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants