Skip to content

Make relative pathing to Absolute pathing in st-lib #6963

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.

Sign up for GitHub

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

Jump to bottom
Merged
sfc-gh-wihuang merged 8 commits into from
Jul 7, 2023
Merged

Make relative pathing to Absolute pathing in st-lib #6963

sfc-gh-wihuang merged 8 commits into from
Jul 7, 2023

Conversation

@willhuang1997
Copy link
Contributor

@willhuang1997 willhuang1997 commented Jul 7, 2023 •
edited by sfc-gh-wihuang
Loading

This PR should likely be reviewed by commits.

Describe your changes

  • Change relative pathing to absolute pathing with aliases

GitHub Issue Link (if applicable)

Testing Plan

  • Explanation of why no additional tests are needed
  • Unit Tests (JS and/or Python)
  • E2E Tests
  • Any manual testing needed?
    manual testing for yarn start and hot reloading

Yarn test is failing but I can fix that in relative/st-lib

Video proof of it working:

Screen.Recording.2023-07-07.at.2.50.09.AM.mov

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@sfc-gh-wihuang sfc-gh-wihuang changed the title Fix/absolute/st lib Make relative pathing to Absolute pathing in st-lib Jul 7, 2023
@sfc-gh-wihuang sfc-gh-wihuang marked this pull request as ready for review July 7, 2023 09:49
@lukasmasuch
Copy link
Collaborator

lukasmasuch commented Jul 7, 2023

Looks good 👍 thanks for doing another iteration. One error that I saw in my editor was this for all @streamlit/lib" imports in app:

image

Using @streamlit/lib/src" here instead seems to solve this.

@sfc-gh-wihuang
Copy link
Contributor

sfc-gh-wihuang commented Jul 7, 2023

Looks good 👍 thanks for doing another iteration. One error that I saw in my editor was this for all @streamlit/lib" imports in app:

image Using `@streamlit/lib/src"` here instead seems to solve this.

I think it's actually reasonable for you. I think you will have to build @streamlit/lib by doing yarn buildLib in the frontend dir. I don't think we actually want to do @streamlit/lib/src because then yarn workspaces won't properly be able to refer to @streamlit/lib as a library.

@sfc-gh-wihuang sfc-gh-wihuang merged commit ea9829c into relative/st-lib Jul 7, 2023
@vdonato vdonato deleted the fix/absolute/st-lib branch November 1, 2023 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lukasmasuch lukasmasuch lukasmasuch approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@willhuang1997 @lukasmasuch @sfc-gh-wihuang