Fix Eslint and Type-checking for st-lib #6675

willhuang1997 · 2023-05-11T21:52:33Z

This PR is to fix eslint for app and lib. This also adds dependencies that are necessary to lib's package.json. This is done through eslint noticing the necessary dependencies. This PR also does some linting and also does a merge from develop as we need the BaseButton refactoring to stop duplication of exports from the root index.ts.

In addition, this PR fixes the type checking github actions by adding new commands.

willhuang1997 · 2023-05-11T23:05:47Z

frontend/lib/src/theme/utils.ts

 import { createBaseUiTheme } from "./createThemeUtil"

 export const AUTO_THEME_NAME = "Use system setting"
 export const CUSTOM_THEME_NAME = "Custom Theme"

+export const getSystemTheme = (): ThemeConfig => {


This needs to be moved before getSystemTheme gets called.

willhuang1997 · 2023-05-11T23:07:48Z

frontend/package.json

+    "lint": "yarn lintApp && yarn lintLib",
+    "lintApp": "yarn workspace @streamlit/app lint",
+    "lintLib": "yarn workspace @streamlit/lib lint",
+    "prettier": "prettier --write --config .prettierrc app/src && prettier --write --config .prettierrc lib/src"


adding this because it's useful but not necessary for linting / typechecking

willhuang1997 · 2023-05-11T23:08:22Z

scripts/validate_frontend_lib_imports.py

deleted because we don't need it

willhuang1997 · 2023-05-11T23:26:23Z

frontend/lib/tsconfig.json

extend root one so less duplication. I tried adding the extends property to app/tsconfig.json but it broke tschecks so will investigate that later.

willhuang1997 · 2023-05-11T23:38:24Z

frontend/lib/package.json

-        "build:vendor": "cp -R ./src/vendor ./dist"
+        "build:vendor": "cp -R ./src/vendor ./dist",
+        "build:assets": "cp -R ./src/assets ./dist",
+        "build:proto": "cp ./src/proto* ./dist",


Need to look into if there's a better way of exporting this.

willhuang1997 · 2023-05-11T23:39:38Z

frontend/lib/package.json

    },
    "dependencies": {
+        "@deck.gl/core": "^8.8.23",


These dependencies should be a minimal set as they are added from eslint errors. app/package.json will need be cleaned up and it's already documented as something to do.

tconkling

Looks great!

tconkling · 2023-05-11T23:39:43Z

Makefile

+.PHONY: frontend-lib
+frontend-lib:
+	cd frontend/ ; yarn run buildLib;
+
+.PHONY: frontend-app
+frontend-app:
+	cd frontend/ ; yarn run buildApp
+


Can you add short docstrings for these rules (just single-line descriptions under the .PHONY line)

tconkling · 2023-05-11T23:43:34Z

frontend/app/package.json

@@ -14,7 +14,7 @@
    "cy:run": "unset NODE_OPTIONS && cypress run",
    "cy:run-flaky": "yarn cy:run --config integrationFolder=../e2e_flaky/specs",
    "typecheck": "tsc --noEmit",
-    "lint": "eslint --ext .js --ext .jsx --ext .ts --ext .tsx --max-warnings 0 --cache --fix src"


is the --fix arg intentionally removed?

Hm that's how it is in develop so I removed. I personally like the --fix when I run it locally and accidentally added it in a separate PR seemingly. I am happy to keep it because I use it but not sure if others use it.

Makes sense - let's keep parity with develop

* Minimal exports for app to consume + VerticalBlock and ElementNodeRenderer (#6623) This PR is the first milestone to determine the minimal number of exports needed for app to consume + VerticalBlock + ElementNodeRenderer. Most of the changes are import path changes. Some of the more notable changes to look through are How the @streamlit/lib is built. It's built through the monorepo config for babel (https://babeljs.io/docs/config-files/#monorepos). a. The babel process works by using a custom preset that is mainly babel-preset-react-app but slightly modified in order to make sure glide data grid code is es6 compatible and hard coding isTypeScriptEnabled to True. b. The other keys are adding in the emotion plugin for styled-components, ignoring proto, vendor, test files, and telling babel that frontend is the root directory for babel config. The actual index.ts that is added to frontend/lib Some things to note that will be punt down but worked on later: We also need to look into why protobuf can't be compiled through tsc. Cleaning frontend/app/package.json Cleaning frontend/app/yarn.lock Cleaning frontend/lib/scripts/create.js Determine what's going wrong with globalStyles typing Determine what's going wrong with styled-components typing for EmotionTheme * Change exports for protobuf to be included in index.ts instead of copied through cp command. (#6635) Tim noticed that I was copying the protobuf.js and protobuf.d.ts instead of compiling it through babel. That causes problems because then you have to import by doing import { ... } from "@streamlit/lib/dist/proto" which doesn't make any sense. The reason why I did that was because I was getting an error on first try: `src/proto.js:4:1 - error TS9005: Declaration emit for this file requires using private name 'Radii'. An explicit type annotation may unblock declaration emit. 4 import * as $protobuf from "protobufjs/minimal"; ~~~~~~ src/proto.js:4:1 - error TS9005: Declaration emit for this file requires using private name 'Radio'. An explicit type annotation may unblock declaration emit. 4 import * as $protobuf from "protobufjs/minimal";` I just removed the proto files and did a make protobuf and then tried exporting and that worked, as it should. * Simple fix for globalStyles (#6651) * Fix typing issues for styled-components (#6652) There are going to be 2 emotion.d.ts. Our @streamlit/lib has our own theming and then our app will simply just be consuming from @streamlit/lib which makes sense. * remove extra app directory in frontend dir (#6653) The current folder structure is frontend/app/src/app. We should make it so that it's frontend/app/src and remove the extra app directory. This PR also fixes the audit_license script. * Remove unnecessary files * Fix naming of directories and files * Fix BaseColorPicker directory * Add Fixes that should exist in feature/st-lib * Fix Eslint and Type-checking for st-lib (#6675) This PR is to fix eslint for app and lib. This also adds dependencies that are necessary to lib's package.json. This is done through eslint noticing the necessary dependencies. This PR also does some linting and also does a merge from develop as we need the BaseButton refactoring to stop duplication of exports from the root index.ts. In addition, this PR fixes the type checking github actions by adding new commands. * Fix notices (#6676) * Clean up audit_frontend_licenses * Fix/yarn test (#6681) This is to fix yarn test. This simply adds new scripts through the yarn workspace command to run app and lib tests. Currently, the lib testing works by mainly using the same scripts that CRA uses for transpiling typescript to javascript. There is one other thing to note: It looks like if you were to upgrade to 17.0.1 supposedly for react-dom, it would fix jest tests for app to not hang (facebook/react#20756 (comment)). However, jest was still hanging so I just did a delete MessageChannel within src/setupTests.js and we can remove that when we upgrade to React 18. * Fix regex that I accidentally forgot to commit (#6698) * Fix/cypress (#6699) * Remove unused app dep / dev dep and declarations.d.ts (#6713) * Remove unused app dep / dev dep and declarations.d.ts * Remove d3 color resolution (#6714) * Add dependencies removed from app to lib/package.json * Add pbjs cli * Readd notices * Add watch lib script (#6730) Since @streamlit/lib is separated out to be its own separate lib, we need a script to constantly rebuild when the files change. Will also need to update the contributing wiki after this is merged to develop. * Get rid of yarn start warnings and clean Makefile (#6739) * Remove streamlit.css. I'm not sure where it came from * Fix yarn buildLib * Fix eslintrc * Fix codeql * Add notices and yarn.lock * Fix glide-data-grid versions * Fix nits * Fix nits * Lint and cleanup * Minor cleanup for Block utils as comments got removed * Remove accidentally committed file * Revert "Remove accidentally committed file" This reverts commit 87b7f13. * Fix yarn lock * Fix compilation errors * Fix lint * Fix notices * Remove unnecessary package.json dependencies * Fix package.json versions * Fix yarn.lock and streamlit dialog snapshots * Fix notices * Have faster hot reloading and absolute imports for @streamlit/lib (#6961)  ## Describe your changes - Change imports for `app` to `@streamlit/app/src...` as we removed the `ModuleScopePlugin` which allows us to import out of `app/src`. - Change imports for `lib` to absolute imports - add `tsconfig.dev.json` that is specifically for developement - change `tsconfig.json` for `lib` and `app` - change `.eslintrc` to `.eslintrc.js` in order to gain access to `__dirname` to make typescript configs relative to each project ## GitHub Issue Link (if applicable) ## Testing Plan Manually testing - Explanation of why no additional tests are needed - Unit Tests (JS and/or Python) - E2E Tests - Any manual testing needed? yes https://github.com/streamlit/streamlit/assets/16749069/cc0a5f96-f8cd-487b-b70e-6a4e5823f0e7 --- **Contribution License Agreement** By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license. * Fix minor comments from Lukas

* Minimal exports for app to consume + VerticalBlock and ElementNodeRenderer (streamlit#6623) This PR is the first milestone to determine the minimal number of exports needed for app to consume + VerticalBlock + ElementNodeRenderer. Most of the changes are import path changes. Some of the more notable changes to look through are How the @streamlit/lib is built. It's built through the monorepo config for babel (https://babeljs.io/docs/config-files/#monorepos). a. The babel process works by using a custom preset that is mainly babel-preset-react-app but slightly modified in order to make sure glide data grid code is es6 compatible and hard coding isTypeScriptEnabled to True. b. The other keys are adding in the emotion plugin for styled-components, ignoring proto, vendor, test files, and telling babel that frontend is the root directory for babel config. The actual index.ts that is added to frontend/lib Some things to note that will be punt down but worked on later: We also need to look into why protobuf can't be compiled through tsc. Cleaning frontend/app/package.json Cleaning frontend/app/yarn.lock Cleaning frontend/lib/scripts/create.js Determine what's going wrong with globalStyles typing Determine what's going wrong with styled-components typing for EmotionTheme * Change exports for protobuf to be included in index.ts instead of copied through cp command. (streamlit#6635) Tim noticed that I was copying the protobuf.js and protobuf.d.ts instead of compiling it through babel. That causes problems because then you have to import by doing import { ... } from "@streamlit/lib/dist/proto" which doesn't make any sense. The reason why I did that was because I was getting an error on first try: `src/proto.js:4:1 - error TS9005: Declaration emit for this file requires using private name 'Radii'. An explicit type annotation may unblock declaration emit. 4 import * as $protobuf from "protobufjs/minimal"; ~~~~~~ src/proto.js:4:1 - error TS9005: Declaration emit for this file requires using private name 'Radio'. An explicit type annotation may unblock declaration emit. 4 import * as $protobuf from "protobufjs/minimal";` I just removed the proto files and did a make protobuf and then tried exporting and that worked, as it should. * Simple fix for globalStyles (streamlit#6651) * Fix typing issues for styled-components (streamlit#6652) There are going to be 2 emotion.d.ts. Our @streamlit/lib has our own theming and then our app will simply just be consuming from @streamlit/lib which makes sense. * remove extra app directory in frontend dir (streamlit#6653) The current folder structure is frontend/app/src/app. We should make it so that it's frontend/app/src and remove the extra app directory. This PR also fixes the audit_license script. * Remove unnecessary files * Fix naming of directories and files * Fix BaseColorPicker directory * Add Fixes that should exist in feature/st-lib * Fix Eslint and Type-checking for st-lib (streamlit#6675) This PR is to fix eslint for app and lib. This also adds dependencies that are necessary to lib's package.json. This is done through eslint noticing the necessary dependencies. This PR also does some linting and also does a merge from develop as we need the BaseButton refactoring to stop duplication of exports from the root index.ts. In addition, this PR fixes the type checking github actions by adding new commands. * Fix notices (streamlit#6676) * Clean up audit_frontend_licenses * Fix/yarn test (streamlit#6681) This is to fix yarn test. This simply adds new scripts through the yarn workspace command to run app and lib tests. Currently, the lib testing works by mainly using the same scripts that CRA uses for transpiling typescript to javascript. There is one other thing to note: It looks like if you were to upgrade to 17.0.1 supposedly for react-dom, it would fix jest tests for app to not hang (facebook/react#20756 (comment)). However, jest was still hanging so I just did a delete MessageChannel within src/setupTests.js and we can remove that when we upgrade to React 18. * Fix regex that I accidentally forgot to commit (streamlit#6698) * Fix/cypress (streamlit#6699) * Remove unused app dep / dev dep and declarations.d.ts (streamlit#6713) * Remove unused app dep / dev dep and declarations.d.ts * Remove d3 color resolution (streamlit#6714) * Add dependencies removed from app to lib/package.json * Add pbjs cli * Readd notices * Add watch lib script (streamlit#6730) Since @streamlit/lib is separated out to be its own separate lib, we need a script to constantly rebuild when the files change. Will also need to update the contributing wiki after this is merged to develop. * Get rid of yarn start warnings and clean Makefile (streamlit#6739) * Remove streamlit.css. I'm not sure where it came from * Fix yarn buildLib * Fix eslintrc * Fix codeql * Add notices and yarn.lock * Fix glide-data-grid versions * Fix nits * Fix nits * Lint and cleanup * Minor cleanup for Block utils as comments got removed * Remove accidentally committed file * Revert "Remove accidentally committed file" This reverts commit 87b7f13. * Fix yarn lock * Fix compilation errors * Fix lint * Fix notices * Remove unnecessary package.json dependencies * Fix package.json versions * Fix yarn.lock and streamlit dialog snapshots * Fix notices * Have faster hot reloading and absolute imports for @streamlit/lib (streamlit#6961)  ## Describe your changes - Change imports for `app` to `@streamlit/app/src...` as we removed the `ModuleScopePlugin` which allows us to import out of `app/src`. - Change imports for `lib` to absolute imports - add `tsconfig.dev.json` that is specifically for developement - change `tsconfig.json` for `lib` and `app` - change `.eslintrc` to `.eslintrc.js` in order to gain access to `__dirname` to make typescript configs relative to each project ## GitHub Issue Link (if applicable) ## Testing Plan Manually testing - Explanation of why no additional tests are needed - Unit Tests (JS and/or Python) - E2E Tests - Any manual testing needed? yes https://github.com/streamlit/streamlit/assets/16749069/cc0a5f96-f8cd-487b-b70e-6a4e5823f0e7 --- **Contribution License Agreement** By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license. * Fix minor comments from Lukas

willhuang1997 added 15 commits May 11, 2023 14:44

Fix dependencies and add new scripts

73f6f97

Fix github actions

43f1872

Add extra comment for variable.scss

cefedd1

Add root tsconfig.json

a294e5a

Move app imgs to lib imgs for balloon and snow

6d3d68a

Remove unnecessary tsconfig key

ea3e2ce

Remove unnecessary script

f19cf5d

Eslint for files

38ee2ec

Add missed element naming change

25c0635

Lint

6b5ea3b

Fix import statement

65142d0

Fix export naming

820ff3b

Move balloons and snow

042f855

Move getSystemTheme to before method is called

6e4af79

Add new lines that should be there

e9a4042

willhuang1997 commented May 11, 2023

View reviewed changes

scripts/validate_frontend_lib_imports.py Outdated

Copy link

Contributor Author

willhuang1997 May 11, 2023

Choose a reason for hiding this comment

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

deleted because we don't need it

Clean package.json and have lib/tsconfig.json consume root tsconfig.json

32c306a

willhuang1997 commented May 11, 2023

View reviewed changes

willhuang1997 marked this pull request as ready for review May 11, 2023 23:27

willhuang1997 requested review from vdonato, mayagbarnes and kmcgrady as code owners May 11, 2023 23:27

willhuang1997 commented May 11, 2023

View reviewed changes

tconkling approved these changes May 11, 2023

View reviewed changes

Add comments for makefile

9b234e0

willhuang1997 merged commit 96c844f into feature/st-lib May 11, 2023

sfc-gh-kmcgrady deleted the fix/EslintAndTS branch October 5, 2023 19:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Eslint and Type-checking for st-lib #6675

Fix Eslint and Type-checking for st-lib #6675

willhuang1997 commented May 11, 2023

willhuang1997 May 11, 2023

willhuang1997 May 11, 2023

willhuang1997 May 11, 2023

willhuang1997 May 11, 2023 •

edited

Loading

willhuang1997 May 11, 2023

willhuang1997 May 11, 2023

tconkling left a comment

tconkling May 11, 2023

tconkling May 11, 2023

willhuang1997 May 11, 2023

tconkling May 11, 2023

Fix Eslint and Type-checking for st-lib #6675

Fix Eslint and Type-checking for st-lib #6675

Conversation

willhuang1997 commented May 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

willhuang1997 May 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tconkling left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

willhuang1997 May 11, 2023 •

edited

Loading