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

Use shared tsconfig from design system #999

Merged
merged 1 commit into from
Nov 29, 2024

Conversation

edlerd
Copy link
Collaborator

@edlerd edlerd commented Nov 22, 2024

Done

  • Use shared tsconfig from design system
  • Update eslint typescript packages
  • Fix code issues to align with new rules.

Fixes WD-17093

@webteam-app
Copy link

@edlerd edlerd force-pushed the shared-tsconfig branch 9 times, most recently from 6e29d32 to 033ef06 Compare November 22, 2024 14:22
@edlerd edlerd marked this pull request as ready for review November 22, 2024 14:23
@edlerd edlerd force-pushed the shared-tsconfig branch 2 times, most recently from 001c7d8 to 3242666 Compare November 22, 2024 14:30
Copy link
Member

@jmuzina jmuzina left a comment

Choose a reason for hiding this comment

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

Very cool to see these configs getting some use, thanks for your work on this! A couple comments below to get the discussion going.

package.json Outdated Show resolved Hide resolved
src/types/cytoscapecomponent.d.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
src/Root.tsx Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
@edlerd edlerd force-pushed the shared-tsconfig branch 3 times, most recently from ad33043 to f84a29e Compare November 22, 2024 22:57
@edlerd
Copy link
Collaborator Author

edlerd commented Nov 22, 2024

Thx for the review and suggestions @jmuzina and @mas-who . Please have another look with the updated config and changes.

tsconfig.json Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
@edlerd edlerd force-pushed the shared-tsconfig branch 3 times, most recently from 82afc87 to eda9eb7 Compare November 27, 2024 11:25
@edlerd edlerd force-pushed the shared-tsconfig branch 3 times, most recently from 143a602 to 9be015f Compare November 27, 2024 11:51
@edlerd
Copy link
Collaborator Author

edlerd commented Nov 27, 2024

Added some custom tsconfig options back and updated the eslint typescript plugins to be compatible with the ts update.
@mas-who and @jmuzina please have another pass at this PR.

@edlerd edlerd requested review from mas-who and jmuzina November 27, 2024 12:05
@@ -13,8 +13,8 @@
"format-js-prettier": "prettier 'src/**/*.{json,jsx,tsx,ts}' 'tests/**/*.ts' --write",
"format-js": "yarn format-js-eslint && yarn format-js-prettier",
"lint-scss": "stylelint 'src/**/*.scss'",
"lint-js-eslint": "eslint 'src/**/*.{json,tsx,ts}' 'tests/**/*.ts' .eslintrc.cjs babel.config.js",
"lint-js-prettier": "prettier 'src/**/*.{json,tsx,ts}' 'tests/**/*.ts' .eslintrc.cjs babel.config.js --check",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume the config files are removed from the script because we updated to the latest version of eslint, and also prettier does not require them?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In order to check them here, we would need to set allowJs in the typescript config, which I'd like to avoid. Hence removing linting for the config files.

tsconfig.json Show resolved Hide resolved
Copy link
Collaborator

@mas-who mas-who left a comment

Choose a reason for hiding this comment

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

LGTM 👍 just had a query regarding the linting scripts, but not blocking at all

@edlerd edlerd merged commit 9d04bbe into canonical:main Nov 29, 2024
11 checks passed
@edlerd edlerd deleted the shared-tsconfig branch January 15, 2025 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants