streamlit / docs

Source code for the Streamlit Python library documentation
https://docs.streamlit.io
Apache License 2.0
110 stars 459 forks source link

"Contributing" instructions for Linux are outdated/incomplete #1104

Closed Dev-iL closed 2 months ago

Dev-iL commented 2 months ago

Link to doc page in question (if any): https://github.com/streamlit/streamlit/wiki/Contributing

Name of the Streamlit feature whose docs need improvement: Contributing Guide - Linux

What you think the docs should say: the instructions provided should be up-to-date and correct.


I would like to contribute some code to Streamlit, but am unable to get a working dev environment after following the instructions.

Here are a few issues I encountered:

  1. Optional "update" step would downgrade node from v22.x to v10.x.
  2. Deprecated argument to yarn --frozen-lockfile doesn't permit node module installation.
  3. protoc binary version not ensured to be correct (if the version is too old, the protobuf stage fails).
  4. Removed yarn --silent flag causes protobuf section in make to fail.
  5. protobuf section in make fails due to missing dependencies. Solution:
    yarn add @babel/cli @babel/core @types/babel__core @types/babel__preset-env @babel/plugin-proposal-class-properties @babel/plugin-proposal-decorators @babel/plugin-proposal-private-methods @babel/plugin-proposal-private-property-in-object @babel/preset-env @babel/preset-react @babel/preset-typescript @pbts/core babel-plugin-module-resolver babel-plugin-transform-react-remove-prop-types pbjs protobufjs-cli

And the 🍒 on top: even after resolving all of the above, a successful make all results in a broken build (where every app one attempts to run, including hello is blank with multiple JS errors and warnings in the browser console).

Tested on: Ubuntu 22.04, Python 3.12.4, Node 22.3.0, Yarn 4.3.1

raethlein commented 2 months ago

@Dev-iL Thanks for pointing out the issues with the current instructions and providing some fixes! I have looked into it today and the root cause of the issue is that yarn version 4.x is installed when following the Linux instructions whereas we still use 1.22.21. This version mismatch leads to a new generated .lock file and the deprecation issues for some flags as you mentioned. I have updated our docs to point out that you have to run

cd frontend/
yarn set version 1.22.21 # maybe has to be executed twice!

to fix the issues. Afterwards, the setup works as expected. I have put an item on our backlog to update the yarn version, but I did not want to encourage using 4.x for Linux while on Mac we still use the older version. The goal is to migrate our setup to the new version and then update the artifacts in one go.

Dev-iL commented 2 months ago

@raethlein Thanks for the extra instructions - this indeed helped!

raethlein commented 2 months ago

Awesome, thanks for letting us know 🙂 As soon as we are migrating the current yarn version, we are going to update our guidelines.

Dev-iL commented 2 months ago

Regarding this issue - would you like to keep it open as a reminder? I don't mind closing it.

BTW, if you're curious what I needed it for - https://github.com/streamlit/streamlit/pull/8952 (reviews welcome!)

sfc-gh-dmatthews commented 2 months ago

Since this has been updated, I'll close the issue.