-
Notifications
You must be signed in to change notification settings - Fork 53
fix: build #471
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
fix: build #471
Conversation
WalkthroughThis change updates build configurations across multiple Dojo Engine packages to address and fix build issues. Adjustments include removing some build scripts in Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant BuildSystem
participant InternalPkg
participant WasmPkg
Developer->>BuildSystem: Initiate build
BuildSystem->>InternalPkg: Run build:internal
BuildSystem->>WasmPkg: Run build:wasm (after build:internal)
Note right of BuildSystem: Dependencies updated to ensure correct order
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
package.json (1)
5-9
: Consider documenting the new script
build:internal
alters the public build surface; update the README / contributor docs to explain when to use it versus the existingbuild
/build:deps
/build:wasm
targets to avoid confusion..changeset/dry-readers-rhyme.md (1)
1-18
: Changeset message is too vague for future consumers
fix: build issues
gives little context once this lands in the changelog. A one-liner describing what kind of build breakage was addressed (e.g. “align package entry points with tsup output” or similar) will be more useful to downstream users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/dry-readers-rhyme.md
(1 hunks)package.json
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: check
- GitHub Check: build
🔇 Additional comments (1)
package.json (1)
6-6
: Confirmed:build:internal
task exists in Turborepo pipeline
- Located in turbo.json at line 15
No further action required.
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
turbo.json (3)
7-10
: Re-evaluatecache:false
+interruptible:true
on thedev
task
Disabling the Turborepo cache means everydev
restart will rebuild all upstreambuild:deps
tasks even when nothing changed, noticeably slowing the feedback loop. Unless you hit correctness issues with stale artifacts, consider keeping the cache on and relying solely oninterruptible:true
to kill in-flight builds when files change.
21-22
: Redundant self-like dependency inbuild:deps
"dependsOn": ["^build:deps", "^build:internal"]
already ensures upstream packages build their deps. Keeping^build:deps
here is fine but can trigger duplicate runs when the graph is large. A leaner alternative:- "dependsOn": ["^build:deps", "^build:internal"], + "dependsOn": ["^build:internal"],
47-48
: Minor:test:watch
setspersistent:true
explicitly
test:watch
inheritspersistent:true
from the task name convention, so the explicit key is harmless but unnecessary noise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
turbo.json
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: check
🔇 Additional comments (2)
turbo.json (2)
11-13
: Verify that adding^build:wasm
tobuild:internal
does not create hidden cycles
build:internal → ^build:wasm
while each package’sbuild:wasm
in turn depends on^build:wasm
can form long dependency chains. Ensure no package contains both scripts, otherwise Turbo may traverse the graph twice or report a loop.
25-25
:build
task now ignores^build:deps
and^build:wasm
– intentional?
If downstream packages rely on those artifacts at publish time, the rootturbo run build
will no longer guarantee they exist. Double-check publishing CI still succeeds after this simplification.
@@ -1,27 +1,28 @@ | |||
{ | |||
"$schema": "./node_modules/turbo/schema.json", | |||
"$schema": "https://turborepo.com/schema.v2.json", |
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.
Incorrect $schema
URL – IDE/tooling autocompletion will break
turbo.build
is the canonical domain for Turbo schemas. The .com
URL currently referenced does not resolve and will disable JSON-Schema validation in many editors.
- "$schema": "https://turborepo.com/schema.v2.json",
+ "$schema": "https://turbo.build/schema.json",
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"$schema": "https://turborepo.com/schema.v2.json", | |
"$schema": "https://turbo.build/schema.json", |
🤖 Prompt for AI Agents
In turbo.json at line 2, the $schema URL is incorrect and points to a
non-resolving .com domain, which breaks IDE autocompletion and JSON schema
validation. Update the $schema value to use the correct canonical domain
"https://turbo.build/schema.v2.json" to restore proper tooling support.
Closes #
Introduced changes
Checklist
Summary by CodeRabbit
Bug Fixes
Chores