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

Centralize tooling configs in monorepo #4138

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

aryaemami59
Copy link
Contributor

@aryaemami59 aryaemami59 commented Jan 29, 2024

This PR:

  • Creates dedicated repos housing shareable config files for:
  • ESLint
  • Prettier
  • TypeScript
  • Vitest

Part 1 of 3

This PR precedes #4606 and #4607.

This is the initial step in a three-part effort to centralize tooling configurations so they can be shared across multiple repos.

Subsequent PRs:

  • Part 2: Adopts the centralized tooling configurations across the codebase.
  • Part 3: Implements the necessary changes to comply with the new centralized configurations.

Copy link

codesandbox bot commented Jan 29, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Copy link

netlify bot commented Jan 29, 2024

Deploy Preview for redux-starter-kit-docs ready!

Name Link
🔨 Latest commit e2401a9
🔍 Latest deploy log https://app.netlify.com/sites/redux-starter-kit-docs/deploys/672b8e0d391d6b00088c8939
😎 Deploy Preview https://deploy-preview-4138--redux-starter-kit-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codesandbox-ci bot commented Jan 29, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit e2401a9:

Sandbox Source
@examples-query-react/basic Configuration
@examples-query-react/advanced Configuration
@examples-action-listener/counter Configuration
rtk-esm-cra Configuration

@aryaemami59 aryaemami59 marked this pull request as ready for review February 8, 2024 13:45
packages/toolkit/src/createAsyncThunk.ts Outdated Show resolved Hide resolved
packages/toolkit/src/combineSlices.ts Outdated Show resolved Hide resolved
packages/toolkit/src/tests/createSlice.test.ts Outdated Show resolved Hide resolved
@markerikson
Copy link
Collaborator

Okay, what I think I'm seeing here is:

  • Added the configs
  • A lot of lint fixes, mostly around auto-sorting of imports and forcing type imports
  • Extracting some reusable TS types like AnyObject

Can I get a summary of what lint rules changed?

Also it looks like there's some commented out lint rules in the ESLint config

@aryaemami59
Copy link
Contributor Author

@markerikson

Sure, so mostly what I did was, add 2 configs:

  • plugin:@typescript-eslint/recommended
  • plugin:@typescript-eslint/stylistic

Along with prettier which turns off ESLint rules that conflict with Prettier.
Then I fixed all the lint issues, and the ones that were not worth fixing (or were too time consuming) I disabled.
As for the commented out rules, I will probably put up another PR after this one, with those rules and configs enabled and make adjustments and fixes as needed. I just did not want to do it all in one PR because this PR is already way too large as it is.

@EskiMojo14
Copy link
Collaborator

could you add a "prepack": "yarn build" script to any of the packages with a build step? it means we can use our existing publish action with them

@aryaemami59
Copy link
Contributor Author

could you add a "prepack": "yarn build" script to any of the packages with a build step? it means we can use our existing publish action with them

Yeah sure!

@aryaemami59 aryaemami59 marked this pull request as draft February 12, 2024 23:22
@aryaemami59 aryaemami59 marked this pull request as ready for review February 24, 2024 12:24
@timdorr
Copy link
Member

timdorr commented Jun 9, 2024

Hey @aryaemami59, is ESLint being run in CI anywhere? I just noticed we're not running it on React Redux and it's been bugged for a while in that repo. Our first job is currently Lint, Test, Build & Pack, but it appears to only be Build & Pack 😬 Luckily we test in the TS matrix, but I don't think ESLint is run anywhere along the way.

@aryaemami59
Copy link
Contributor Author

@timdorr yeah I kinda knew we weren't running ESLint during CI, I think we're only running it for Reselect. Honestly my plan was to add it to CI after we take care of this one. If you want I can take care of it tomorrow.

@markerikson
Copy link
Collaborator

markerikson commented Aug 11, 2024

Can we separate out these changes into multiple PRs?

  • One to add all the new tooling configs
  • Second to actually switch to using the configs for testing and such
  • And a third to apply just any formatting changes

Also, why is yarn-4.1.0.cjs getting changed? Did it get formatted or something?

@aryaemami59 aryaemami59 force-pushed the configs branch 2 times, most recently from fdec05d to 6554bdf Compare August 13, 2024 16:10
@aryaemami59
Copy link
Contributor Author

I'm gonna split this into multiple PRs soon.

@aryaemami59 aryaemami59 force-pushed the configs branch 2 times, most recently from edb6f9d to 46238ff Compare August 27, 2024 16:08
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.

4 participants