Skip to content

Commit

Permalink
Feature/st lib (streamlit#6692)
Browse files Browse the repository at this point in the history
* 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)

<!--
⚠️ BEFORE CONTRIBUTING PLEASE READ OUR CONTRIBUTING GUIDELINES!
https://github.com/streamlit/streamlit/wiki/Contributing
-->

## 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
  • Loading branch information
willhuang1997 authored and eric-skydio committed Dec 20, 2023
1 parent b8a10db commit 418e3c2
Show file tree
Hide file tree
Showing 720 changed files with 7,075 additions and 5,094 deletions.
2 changes: 1 addition & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ scripts/audit_frontend_licenses.py @vdonato @kmcgrady @tconkling
# to be extra-careful with these.
proto/ @vdonato @tconkling @LukasMasuch @sfc-gh-mnowotka
lib/streamlit/web/server/server.py @vdonato @tconkling @LukasMasuch @sfc-gh-mnowotka
frontend/src/lib/DefaultStreamlitEndpoints.ts @vdonato @tconkling @LukasMasuch @sfc-gh-mnowotka
frontend/lib/src/DefaultStreamlitEndpoints.ts @vdonato @tconkling @LukasMasuch @sfc-gh-mnowotka
7 changes: 4 additions & 3 deletions .github/codeql-config.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
paths:
- lib/streamlit
- frontend/src
- frontend/lib/src
- frontend/app/src
paths-ignore:
- "frontend/src/lib/vendor/**"
- "frontend/src/app/vendor/**"
- "frontend/lib/src/vendor/**"
- "frontend/app/src/vendor/**"
4 changes: 4 additions & 0 deletions .github/workflows/cypress.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ jobs:
sudo apt install -y xvfb libgtk2.0-0 libnotify-dev libgconf-2-4 libnss3 libxss1 libasound2 curl
- name: Run make develop
run: make develop
- name: Run make protobuf
run: make protobuf
- name: Run make frontend-lib
run: make frontend-lib
- name: Register Streamlit User & Mapbox Token
run: |
if [ ! -d $HOME/.streamlit ] ; then
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/js-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ jobs:
uses: ./.github/actions/make_init
- name: Run make develop
run: make develop
- name: Run make protobuf
run: make protobuf
- name: Run make frontend-lib
run: make frontend-lib
- name: Audit frontend licenses
run: ./scripts/audit_frontend_licenses.py
- name: Run linters
Expand Down
4 changes: 2 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ venv
# Autogenerated Protobufs
lib/streamlit/proto/*_pb2.py
lib/streamlit/proto/*_pb2.pyi
frontend/src/lib/proto.js
frontend/src/lib/proto.d.ts
frontend/lib/src/proto.js
frontend/lib/src/proto.d.ts

########################################################################
# OSX - https://github.com/github/gitignore/blob/master/Global/macOS.gitignore
Expand Down
20 changes: 15 additions & 5 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,20 @@ repos:
pass_filenames: false
# Typecheck will be run on CI using --hook-stage manual flag,
# check out .github/workflows/js-tests.yml "Run linters" step for details.
- id: typecheck
name: Typecheck
- id: typecheck-app
name: Typecheck App
always_run: true
entry: yarn --cwd frontend tsc --noEmit
entry: yarn --cwd frontend/app tsc --noEmit
files: \.(js|jsx|ts|tsx)$
language: node
pass_filenames: false
stages:
- manual
- id: typecheck-lib
name: Typecheck Lib
always_run: true
# no noEmit flag bc tsconfig.json emitDeclaration key is used
entry: yarn --cwd frontend/lib tsc
files: \.(js|jsx|ts|tsx)$
language: node
pass_filenames: false
Expand Down Expand Up @@ -116,7 +126,7 @@ repos:
/vendor/
|^vendor/
|^component-lib/declarations/apache-arrow
|^frontend/src/assets/css/variables\.scss
|^frontend/app/src/assets/css/variables\.scss
- id: insert-license
name: Add license for all Proto files
files: \.proto$
Expand Down Expand Up @@ -181,7 +191,7 @@ repos:
- id: trailing-whitespace
exclude: |
(?x)
^frontend/src/assets/
^frontend/app/src/assets/
|^NOTICES$
|^proto/streamlit/proto/openmetrics_data_model.proto$
|\.snap$
60 changes: 41 additions & 19 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -211,12 +211,13 @@ clean:
rm -f lib/streamlit/proto/*_pb2.py*
rm -rf lib/streamlit/static
rm -f lib/Pipfile.lock
rm -rf frontend/build
rm -rf frontend/app/build
rm -rf frontend/node_modules
rm -rf frontend/test_results
rm -f frontend/src/lib/proto.js
rm -f frontend/src/lib/proto.d.ts
rm -f frontend/lib/src/proto.js
rm -f frontend/lib/src/proto.d.ts
rm -rf frontend/public/reports
rm -rf frontend/lib/dist
rm -rf ~/.cache/pre-commit
find . -name .streamlit -type d -exec rm -rfv {} \; || true
cd lib; rm -rf .coverage .coverage\.*
Expand Down Expand Up @@ -257,14 +258,14 @@ protobuf: check-protoc
yarn --silent pbjs \
../proto/streamlit/proto/*.proto \
-t static-module --wrap es6 \
) > ./src/lib/proto.js
) > ./lib/src/proto.js

@# Typescript type declarations for our generated protobufs
cd frontend/ ; ( \
echo "/* eslint-disable */" ; \
echo ; \
yarn --silent pbts ./src/lib/proto.js \
) > ./src/lib/proto.d.ts
yarn --silent pbts ./lib/src/proto.js \
) > ./lib/src/proto.d.ts

.PHONY: react-init
react-init:
Expand All @@ -274,26 +275,35 @@ react-init:
react-build:
cd frontend/ ; yarn run build
rsync -av --delete --delete-excluded --exclude=reports \
frontend/build/ lib/streamlit/static/
frontend/app/build/ lib/streamlit/static/

.PHONY: frontend-fast
# Build frontend into static files faster by setting BUILD_AS_FAST_AS_POSSIBLE=true flag, which disables eslint and typechecking.
frontend-fast:
cd frontend/ ; yarn run buildFast
rsync -av --delete --delete-excluded --exclude=reports \
frontend/build/ lib/streamlit/static/
frontend/app/build/ lib/streamlit/static/

.PHONY: frontend-lib
# Build the frontend library
frontend-lib:
cd frontend/ ; yarn run buildLib;

.PHONY: frontend-app
# Build the frontend app. One must build the frontend lib first before building the app.
frontend-app:
cd frontend/ ; yarn run buildApp

.PHONY: jslint
# Lint the JS code
jslint:
./scripts/validate_frontend_lib_imports.py frontend/src/lib
cd frontend; \
yarn lint;

.PHONY: tstypecheck
# Type check the JS/TS code
tstypecheck:
pre-commit run typecheck --all-files --hook-stage manual
pre-commit run typecheck-lib --all-files --hook-stage manual && pre-commit run typecheck-app --all-files --hook-stage manual

.PHONY: jsformat
# Fix formatting issues in our JavaScript & TypeScript files.
Expand Down Expand Up @@ -335,15 +345,22 @@ notices:
cd frontend; \
yarn licenses generate-disclaimer --silent --production --ignore-platform > ../NOTICES

./scripts/append_license.sh frontend/src/assets/fonts/Source_Code_Pro/Source-Code-Pro.LICENSE
./scripts/append_license.sh frontend/src/assets/fonts/Source_Sans_Pro/Source-Sans-Pro.LICENSE
./scripts/append_license.sh frontend/src/assets/fonts/Source_Serif_Pro/Source-Serif-Pro.LICENSE
./scripts/append_license.sh frontend/src/assets/img/Material-Icons.LICENSE
./scripts/append_license.sh frontend/src/assets/img/Open-Iconic.LICENSE
./scripts/append_license.sh frontend/src/lib/vendor/bokeh/bokeh-LICENSE.txt
./scripts/append_license.sh frontend/src/lib/vendor/twemoji-LICENSE.txt
./scripts/append_license.sh frontend/src/app/vendor/Segment-LICENSE.txt
./scripts/append_license.sh frontend/src/lib/vendor/react-bootstrap-LICENSE.txt
@# When `yarn licenses` is run in a yarn workspace, it misnames the project as
@# "WORKSPACE AGGREGATOR 2B7C80A7 6734 4A68 BB93 8CC72B9A5DEA". We fix that here.
@# There also isn't a portable way to invoke `sed` to edit files in-place, so we have
@# sed create a NOTICES.bak backup file that we immediately delete afterwards.
sed -i'.bak' 's/PORTIONS OF THE .*PRODUCT/PORTIONS OF THE STREAMLIT PRODUCT/' NOTICES
rm -f NOTICES.bak

./scripts/append_license.sh frontend/app/src/assets/fonts/Source_Code_Pro/Source-Code-Pro.LICENSE
./scripts/append_license.sh frontend/app/src/assets/fonts/Source_Sans_Pro/Source-Sans-Pro.LICENSE
./scripts/append_license.sh frontend/app/src/assets/fonts/Source_Serif_Pro/Source-Serif-Pro.LICENSE
./scripts/append_license.sh frontend/app/src/assets/img/Material-Icons.LICENSE
./scripts/append_license.sh frontend/app/src/assets/img/Open-Iconic.LICENSE
./scripts/append_license.sh frontend/lib/src/vendor/bokeh/bokeh-LICENSE.txt
./scripts/append_license.sh frontend/lib/src/vendor/twemoji-LICENSE.txt
./scripts/append_license.sh frontend/app/src/vendor/Segment-LICENSE.txt
./scripts/append_license.sh frontend/lib/src/vendor/react-bootstrap-LICENSE.txt
./scripts/append_license.sh lib/streamlit/vendor/ipython/IPython-LICENSE.txt

.PHONY: headers
Expand Down Expand Up @@ -391,3 +408,8 @@ connect-test-env:
.PHONY: pre-commit-install
pre-commit-install:
pre-commit install

# Watch the frontend lib for changes and run yarn buildLib whenever a change is detected
.PHONY: watch-frontend-lib
watch-frontend-lib:
python scripts/watch_frontend_lib.py

0 comments on commit 418e3c2

Please sign in to comment.