-
Notifications
You must be signed in to change notification settings - Fork 9
Add devcontainer configuration #226
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
base: main
Are you sure you want to change the base?
Conversation
|
|
FYI it's trivial to add a binding that connects the devcontainer to any folder on host machine. It's not included here, but I can add instructions and a commented example if you think it'd be worthwhile. That's how I'm able to run core in dev mode and test it out on another repo on my machine. |
theoephraim
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for setting this up. I have not started using devcontainers, but definitely seems useful, especially for occasional contributors.
|
|
||
| #### 1. `Cannot find module '@dmno/tsconfig/tsconfig.node.json'` | ||
|
|
||
| **Cause**: This happens when packages with workspace dependencies try to build before the workspace is properly linked. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Im not quite sure I understand what's going on here. If you run pnpm install within the workspace, shouldn't everything get symlinked properly?
| - `@dmno/vercel-platform` | ||
| - `@dmno/cloudflare-platform` | ||
|
|
||
| **Build Rule**: Always build packages in dependency order. Configuration packages first, then core libraries, then everything else. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully folks wont have to think about this too much. A single run of pnpm run build:libs uses turborepo to build everything in the correct dependency order, and then you can work on individual packages.
|
|
||
| # Install latest pnpm using corepack | ||
| echo "📦 Installing latest pnpm using corepack..." | ||
| if sudo corepack prepare pnpm@latest --activate; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than installing the latest, packageManager field in the root package.json should already be respected by corepack. Could be useful to somehow eagerly install it here though?
|
|
||
| # Install common build tools globally | ||
| echo "🔧 Installing global build tools..." | ||
| if sudo npm install -g turbo@latest tsup@latest typescript@latest; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume you are installing these globally just for convenience of writing turbo ... instead of pnpm exec turbo ...?
Doesn't this mean the versions in the root package.json will not be respected, and it would actually use a different installed copy when executing via pnpm versus not?
Maybe there is a way to just alias these commands to the version in node_modules/.bin?
| "build": "pnpm run clean && pnpm run build:injector && pnpm run build:injector:edge && tsup", | ||
| "build:injector": "tsup --config tsup.inject-standalone.config.ts", | ||
| "build:injector:edge": "DMNO_EDGE_COMPAT=1 tsup --config tsup.inject-standalone.config.ts", | ||
| "build": "pnpm run clean && pnpm run build:injector && pnpm run build:injector:edge && NODE_OPTIONS=\"--max-old-space-size=7168\" tsup", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this is to mitigate issues when running within the devcontainer as I have not had any issues locally.
Would it make sense to set them within the devcontainer as NODE_OPTIONS env var?
|
|
||
| Understanding the dependency order helps with troubleshooting: | ||
|
|
||
| ### Configuration Packages (no dependencies) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only concern here is keeping this info up to date as it is written in many places. Obviously AI helps a lot, but maybe better to just include a link to somewhere else?
Avoid "it works on my machine" with devcontainers. They're optional so folks can opt not to use them, but for those that do, they're an awesome way to accelerate the time-to-first-commit because infrastructure configuration is handled.
If you decide you want to go down this path of experiencing the devex, I'd suggest Moon + Proto, Nx, or something like that.