Skip to content

refactor(deps): drop cocoa/objc for objc2-app-kit - #675

Open
Alex Shabalin (alex-sparus) wants to merge 1 commit into
mainfrom
alex/drop-deprecated-deps
Open

refactor(deps): drop cocoa/objc for objc2-app-kit#675
Alex Shabalin (alex-sparus) wants to merge 1 commit into
mainfrom
alex/drop-deprecated-deps

Conversation

@alex-sparus

Copy link
Copy Markdown
Contributor

Summary

Problem: tooling flags block 0.1.6 as unmaintained. Nothing uses it directly; it arrives only through cocoa 0.26. cocoa and objc 0.2 are deprecated upstream as well — no soundness fixes, no bindings for new macOS SDKs, and untyped msg_send! calls no compiler checks.

Solution: move the three NSWindow call sites onto objc2-app-kit, already in the tree through Tauri, so the lockfile gains nothing. The two identical panel-setup blocks in peek.rs collapse into one helper whose main-thread requirement is asserted rather than assumed, and the hardcoded level literal becomes NSFloatingWindowLevel. cocoa, cocoa-foundation, objc, block, malloc_buf and core-graphics 0.24 leave Cargo.lock.

Refs #546

Test Plan

  • No test plan needed
  • Manual smoke testing

Docs

  • Docs updated (companion PR in darkmatter/nixmac-web: #___)
  • No docs update needed

@darkmatter

darkmatter Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

🎨 Storybook preview

Open Storybook preview

Updated for 85f72cc


⚠️ Detected UI changes (5)

These stories' HTML snapshots changed. I've added screenshots + links to the changed stories below. Review them carefully then accept the changes to regenerate baselines and include them in this PR:

Flows/Evolve › Playground

Flows/Evolve › Playground

Flows/Evolve › 1. Begin (idle)

Flows/Evolve › 1. Begin (idle)

Flows/Evolve › 2. Evolving (progress)

Flows/Evolve › 2. Evolving (progress)

Flows/Evolve › Evolving With Error Event

Flows/Evolve › Evolving With Error Event

Flows/Evolve › 3. Review (changes generated)

Flows/Evolve › 3. Review (changes generated)


Accept UI changes

  • Click here to accept these changes

Alternatively, you can run bun run test:update-snapshots locally to re-generate the baselines and then push the changes to this PR.

What does this do?

The screenshots above show UI changes detected by the Storybook
snapshot tests run on this PR. Each image is the rendered output of
a Storybook story from the code in this PR branch; the snapshot
test compared it against the committed baseline in
__snapshots__/ and flagged the difference.

Checking the box tells the darkmatter[bot] to regenerate the
baselines from this PR's current code and commit them directly to
this branch. The new baselines become the source of truth for
future runs — only accept after confirming the visual changes are
intentional.

Comparison baseline: the committed __snapshots__/ files on this
PR branch (carried forward from develop). Accept updates them in
place on this branch.

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor
Warnings
⚠️

No Linear issue ID found in this PR's title, description, or branch name (expected something like ENG-123). Add one so this work is traceable in Linear, or add #no-linear to the PR description to acknowledge it's intentionally untracked.

📋 PR Overview

Lines changed 206 (+60 / -146)
Files 0 added, 4 modified, 0 deleted
Draft / WIP no
Has Test Plan yes
Linear issue no
No Test Plan Needed no
New UI components no
New Storybook stories no
New Rust modules no
New TS source files no
New tests no
package.json touched no
Cargo.toml touched yes
Infra / CI touched no

🔬 Coverage

Report Lines Statements Functions Branches
apps/native/coverage/coverage-summary.json 34.3% 33.8% 29.0% 29.2%

Generated by 🚫 dangerJS against 85f72cc

@czxtm

Copy link
Copy Markdown
Member

darkmatteragent please review this PR

@czxtm

Copy link
Copy Markdown
Member

darkmatteragent please review this PR now

@darkmatteragent darkmatteragent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review — COMMENT

56dc7e8ecac5 · 1 finding

Behavior-preserving swap of three NSWindow call sites from deprecated cocoa/objc to typed objc2-app-kit; sound to merge. Verified upstream: NSFloatingWindowLevel is exactly 3 (the old literal), every method used is ungated under the declared features (NSResponder+NSWindow+objc2-core-foundation; bitflags comes transitively), and the Cargo.lock deletions prove no other workspace crate consumed cocoa/objc.

One doc-only nit: the new SAFETY comments state the wrong ownership mechanism for the ns_window() pointer.

Findings

Tip

nit · documentation — SAFETY comments misdescribe ns_window() as retained-then-autoreleased; tao actually retains it for the window's lifetime
apps/native/src-tauri/src/peek.rs:494

Gutter 494-496 (and the identical comment at main.rs 945-948) claims "ns_window() hands back a retained-then-autoreleased NSWindow, valid for this scope". That mechanism is wrong: tao's UnownedWindow::ns_window() returns &*self.ns_window as *const NSWindow as *mut _ — a plain borrow of the Retained<NSWindow> tao owns, which stays alive for the window's lifetime and is never autoreleased. T

@darkmatteragent

darkmatteragent commented Aug 31, 2026

Copy link
Copy Markdown
Reviewer · darkmatter agent VERDICT
APPROVE
Approve: mechanical, behavior-preserving migration of three NSWindow call sites from deprecated cocoa/objc onto typed objc2-app-kit 0.3.2. - Verified against upstream 0.3.2: NSFloatingWindowLevel = 3 matches the removed literal, and the three collection-behavior flags match cocoa's values bit-for-bit. - Declared features cover every gate the used symbols carry (NSWindow class needs NSResponder+NSWindow; alphaValue needs objc2-core-foundation); lockfile removals leave no dangling dependents. - Consolidated helper preserves old call ordering at both sites; the new debug_assert strengthens, not weakens, the main-thread contract.

No findings.

reviewed: 56dc7e8 — 2026-09-01T16:42Z verdict: approve findings: 0

Reviewer avatar Reviewer
darkmatter agent
Run details
Metadata Value
Agent darkmatter/agent/reviewer
Session pr:darkmatter/nixmac#675
Kind pr
Runtime platform-runtime
Configured model litellm/glm-5.2-fp8
Target darkmatter/nixmac#675
Revision 56dc7e8ecac5b42a630ff70ebcdc573bd06cf143
Verdict approve
Reviewed at 2026-09-01T16:42Z
Logs Open Braintrust session

@alex-sparus

Alex Shabalin (alex-sparus) commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Re the review finding that the SAFETY comments misdescribe ns_window(): the comments are correct as written; the finding traced the wrong crate.

tao::UnownedWindow::ns_window() does return a plain borrow of tao's Retained<NSWindow> — but that method is not in the call path. Both call sites are on tauri::WebviewWindow, and tauri implements ns_window() independently: tauri-2.11.3/src/window/mod.rs:1632 goes through raw_window_handle, gets the NSView, calls view.window() (a fresh Retained<NSWindow>), and returns objc2::rc::Retained::autorelease_ptr(ns_window) — retained-then-autoreleased, exactly as the comments state.

Verified against the registry sources for tao-0.35.3 and tauri-2.11.3, both matching Cargo.lock. No change made.

ping darkmatteragent

@alex-sparus

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by jj-ryu.

@darkmatteragent darkmatteragent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review — APPROVE

56dc7e8ecac5 · 0 findings

Approve: mechanical, behavior-preserving migration of three NSWindow call sites from deprecated cocoa/objc onto typed objc2-app-kit 0.3.2. - Verified against upstream 0.3.2: NSFloatingWindowLevel = 3 matches the removed literal, and the three collection-behavior flags match cocoa's values bit-for-bit. - Declared features cover every gate the used symbols carry (NSWindow class needs NSResponder+NSWindow; alphaValue needs objc2-core-foundation); lockfile removals leave no dangling dependents. - Consolidated helper preserves old call ordering at both sites; the new debug_assert strengthens, not weakens, the main-thread contract.

No findings.

@linear-code

linear-code Bot commented Sep 2, 2026

Copy link
Copy Markdown

ENG-596

Problem: tooling flags `block 0.1.6` as unmaintained. Nothing uses it
directly; it arrives only through `cocoa 0.26`. `cocoa` and `objc 0.2`
are deprecated upstream as well — no soundness fixes, no bindings for
new macOS SDKs, and untyped `msg_send!` calls no compiler checks.

Solution: move the three NSWindow call sites onto `objc2-app-kit`,
already in the tree through Tauri, so the lockfile gains nothing. The
two identical panel-setup blocks in `peek.rs` collapse into one helper
whose main-thread requirement is asserted rather than assumed, and the
hardcoded level literal becomes `NSFloatingWindowLevel`. `cocoa`,
`cocoa-foundation`, `objc`, `block`, `malloc_buf` and
`core-graphics 0.24` leave `Cargo.lock`.

Refs #546
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.

Migrate off unmaintained cocoa/objc crates (drops block 0.1.6)

3 participants