Skip to content

Preserve QFN thermal vias during CLI footprint imports - #4270

Open
techmannih wants to merge 2 commits into
mainfrom
io
Open

Preserve QFN thermal vias during CLI footprint imports#4270
techmannih wants to merge 2 commits into
mainfrom
io

Conversation

@techmannih

Copy link
Copy Markdown
Member

No description provided.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Comment thread scripts/bun-build.ts
)
const manifoldWasm = Bun.file(manifoldWasmSourcePath)
await Bun.write("./dist/cli/manifold.wasm", manifoldWasm)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

circuit-json-to-footprinter@0.0.46 introduces @tscircuit/manifold-2d for footprint geometry operations.

The CLI bundles this dependency into dist/cli/main.js. In Node.js, Manifold loads manifold.wasm relative to that bundled JS file, but Bun does not automatically copy the WASM asset into dist.

As a result, the globally installed CLI crashed during startup because it expected:

dist/cli/manifold.wasm

Copying the package-exported WASM file next to main.js preserves Manifold's runtime loading contract. Only dist/cli needs the asset because the other generated bundles do not include the Manifold loader.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i don't think this is necessary

@github-actions

Copy link
Copy Markdown
Contributor

This PR has been automatically marked as stale because it has had no recent activity. It will be closed if no further activity occurs.

@techmannih

Copy link
Copy Markdown
Member Author

sctive

@imrishabh18 imrishabh18 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not sure, get a review from Seve

@seveibar seveibar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks like some crazy manifold hack was introduced, afaik you don't need this hack

@techmannih

Copy link
Copy Markdown
Member Author

Yes but the built CLI demonstrably crashes without it

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.

3 participants