Use Preconstruct to manage monorepo #285
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
https://monorepo.guide/getting-started suggests that apps directory should contain "user-facing apps and websites" and packages directory should contain "packages designed to be consumed by other packages (published OR internal)". I'm not sure if this is a common convention, but it lookes simple and clear enough for me. Rename our nextjs app to @thunderstore/nextjs and components package to @thunderstore/components. Fix imports to reflect the changes. Update the references in GitHub Actions files to reflect the new file paths.
Basically the changes done here are the results of following this tutorial: https://monorepo.guide/getting-started . Additionally some changes are from an earlier pr (#37), which was also about setting up Preconstruct. * Add Preconstruct as dependency * Setup Babel and ESLint * Rename workspaces to use the structure used in the tutorial. I don't know if this is some convention oslt, but it seems like a nice, clear structure for the repo * Add a script for using Preconstruct, and a postinstall hook to run it automatically while devving (see root package.json) * Move all devDependencies to dependencies on root package.json. This was recommended by Preconstruct, but I can't quite recall why
Update the action to read all versions from dependencies, since the root package.json no longer contains a devDependencies section. Update the packages in the action to use the same versions that are used in the actual repo, and then update the Docker image to use the same Node version.
Introducing Preconstruct caused some weird issues. Whenever "react-markdown" or "remark-gfm" is imported in a normal way, the app crashes with "Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: undefined". Furthermore, the error only occurs after a full page refresh - regular imports don't cause problems when changes are updated by live-reload. The same error occurs with the package versions used before this PR as well as with the latest ones.
As far as I can tell, the recommendation is that each app and package in the monorepo defines all the dependencies it uses. Yarn workspaces will then handle hoisting and deduplicating the dependencies. Root level package.json should contain the tooling dependencies used by the project as a whole. Also I believe this only matters in cases where packages are going to be published separately. I didn't manage to find any solid information on this.
MythicManiac
approved these changes
Nov 30, 2021
There was a problem hiding this comment.
Looks good to me, nice work 👍
Should mention merging this closes #37 (in the PR description) so that indeed happens when this is merged.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR intends to do the same things as PR #37. I initially started to work with it, rebasing it to current master etc., but I encountered one error after another and finally decided to start from the scratch.
Basically I followed this tutorial and fixed any problems I detected. Some of them were already solved in the earlier PR, so the solutions look somewhat similar.
Closes #37