Skip to content

Conversation

@kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Feb 9, 2026

Motivation

Refactoring to unlock #598

Changes

We had been having closure-specific lift/lower logic for JS glue codegen:

  • closureLiftParameter
  • closureLowerReturn
  • closureLowerParameter
  • closureLiftReturn

However, they are essentially the same as the regular lift/lower logic, so we should uniformly use them to reduce the cost of adding a new type support.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors BridgeJS closure-related JS glue generation to reuse the shared lift/lower thunk builders, aiming to unify how Swift closures and JS callbacks are bridged.

Changes:

  • Replaced bespoke closure parameter/return lift/lower code with ImportedThunkBuilder / ExportedThunkBuilder-based generation.
  • Added hasDirectAccessToSwiftClass to IntrinsicJSFragment.PrintCodeContext to control whether generated JS can reference Swift heap object classes directly vs via _exports[...].
  • Updated BridgeJSLink test snapshots for closure bridging (SwiftClosure*.js) to match the new generation.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
Plugins/BridgeJS/Tests/BridgeJSToolTests/Snapshots/BridgeJSLinkTests/SwiftClosureImports.js Snapshot update for closure import glue output.
Plugins/BridgeJS/Tests/BridgeJSToolTests/Snapshots/BridgeJSLinkTests/SwiftClosure.js Snapshot update for closure glue output; exposes an optional Swift heap object construction scoping issue.
Plugins/BridgeJS/Sources/BridgeJSLink/JSGlueGen.swift Introduces hasDirectAccessToSwiftClass and uses it when emitting Swift heap object construction in lift fragments.
Plugins/BridgeJS/Sources/BridgeJSLink/BridgeJSLink.swift Refactors closure invoke/lower generation to use shared thunk builders; adjusts thunk builder function rendering API.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@kateinoigakukun kateinoigakukun force-pushed the katei/18a5-bridgejs-unify-g branch from 7311617 to f90d918 Compare February 9, 2026 13:22
@krodak krodak merged commit 50ad37a into main Feb 9, 2026
11 checks passed
@krodak krodak deleted the katei/18a5-bridgejs-unify-g branch February 9, 2026 16:49
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.

2 participants