Skip to content

fix: add build generated files to *ignore config#41826

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.

Sign up for GitHub

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
okwasniewski wants to merge 1 commit into from
Closed

fix: add build generated files to *ignore config #41826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .eslintignore
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,8 @@ packages/react-native/Libraries/Renderer/*
packages/react-native/Libraries/vendor/**/*
node_modules/
packages/*/node_modules
packages/*/dist

@huntiehuntieDec 7, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As a note, this hasn't been added globally because: 1/ not all of our packages follow this pattern yet, 2/ ideally (and open to debate!) even though we're in a monorepo, packages should self-specify their paths (e.g. in .gitignore, package.json#files), as if they were isolated.

However, I see there is a functional need to set this in .flowconfig and .prettierignore.

Requesting PR changes, to either:

  • Include .flowconfig and .prettierignore changes but drop this entry (personal preference but again open to debate).
  • Leave this entry included, but delete redundant .gitignore entries that are covered by this rule in each package.

@okwasniewskiokwasniewskiDec 7, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Packages that self-specify their ignore paths would be ideal however, eslint doesn't support that (eslint/eslint#13389, neither do prettier: prettier/prettier#4081).

If we delete this entry you are unable to run yarn lint locally, that's the output for me:

✖ 710 problems (18 errors, 692 warnings)
18 errors and 655 warnings potentially fixable with the `--fix` option

And if we delete .gitignore from packages/ we will add dist folders to source control. If you don't want to duplicate this maybe we could remove packages .gitignore and move it to global .gitignore?

@huntiehuntieDec 7, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah my bad! I'm in line with the above 👍🏻👍🏻

I missed that this is .eslintignore and not .gitignore 😅

packages/debugger-frontend/dist/**/*
packages/react-native-codegen/lib
tools/eslint/rules/sort-imports.js
**/Pods/*
3 changes: 3 additions & 0 deletions .flowconfig
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,9 @@
; Checked-in build output
<PROJECT_ROOT>/packages/debugger-frontend/dist/

; Generated build output
<PROJECT_ROOT>/packages/.*/dist

[untyped]
.*/node_modules/@react-native-community/cli/.*/.*

Expand Down
3 changes: 3 additions & 0 deletions .prettierignore
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,3 +4,6 @@
# Ignore hermes as it is downloaded from the react_native_pods
**/sdks/hermes
**/sdks/downloads

packages/*/dist
vendor