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

Switch to using monorepo #4

Merged
merged 7 commits into from
Jul 3, 2023
Merged

Switch to using monorepo #4

merged 7 commits into from
Jul 3, 2023

Conversation

denizenging
Copy link
Member

After countless of hours of research, I think I found the best approach (for me) to implement monorepos using already-available tooling (pnpm in this case).

With this branch, we are now using a much stricter typescript with jest and workspaces support (used to implement the monorepo) along with shared dependencies of the same version (inspired by create-react-app).

I changed a few things that might be controversial. Please comment on them as well. ^^

The files modules shared/ are already laid out well.  Putting them in
to the same place makes little sense to me.
@denizenging denizenging requested a review from puria July 3, 2023 21:18
@denizenging denizenging force-pushed the sf/monorepo branch 2 times, most recently from c2ebf8d to 68beb8a Compare July 3, 2023 22:06
@puria
Copy link
Member

puria commented Jul 3, 2023

I see all your decisions, here. Good paths and choices are taken imho.
Simplest solutions and minimal architecture are always encouraged.

Please do not underestimate the "this also should work on browser" thing, specially for the next packages!

Overall fine for me... I can actually merge it, so we can move on \o/

@puria puria merged commit aa025e5 into main Jul 3, 2023
5 checks passed
@puria puria deleted the sf/monorepo branch July 3, 2023 22:35
Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, thank goodness!

@denizenging
Copy link
Member Author

Thanks! I'll also add support for browsers when the time comes, but it seems rather unnecessary to have unused scripts and files for browsers at this very moment, since we don't produce anything for browsers yet.

I didn't mention (because I forgot :/), but you can bring the project up using pnpm m i (short for pnpm recursive install).

Copy link
Member Author

@denizenging denizenging left a comment

Choose a reason for hiding this comment

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

Hehe. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't get this. What exactly does it do?

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like nothing special. Just related to monorepos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants