New plugins - #10
Merged
Merged
Conversation
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe pull request adds the ChangesRepository tooling
ns-rspack package
NativeScript test app
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant NativeScriptApp
participant MainPage
participant WidgetsGalleryModel
participant TabsPage
NativeScriptApp->>MainPage: navigate to main page
MainPage->>WidgetsGalleryModel: create binding context
MainPage->>WidgetsGalleryModel: dispatch widget interaction
WidgetsGalleryModel->>TabsPage: open tab page
TabsPage->>MainPage: navigate back or close modal
``
</details>
<!-- walkthrough_end -->
<!-- pre_merge_checks_walkthrough_start -->
<details>
<summary>🚥 Pre-merge checks | ✅ 4 | ❌ 1</summary>
### ❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
| :----------------: | :--------- | :------------------------------------------------------------------------------------ | :--------------------------------------------------------------------------------- |
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 61.90% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
<details>
<summary>✅ Passed checks (4 passed)</summary>
| Check name | Status | Explanation |
| :------------------------: | :------- | :------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately identifies the addition of new plugins, which is a major part of the changeset, although it does not specify the Rspack and WASM plugins. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
</details>
</details>
<!-- pre_merge_checks_walkthrough_end -->
<!-- finishing_touch_checkbox_start -->
<details>
<summary>✨ Finishing Touches 💡 1</summary>
<!-- finishing_touch_suggestion:fix_ci -->
<details open>
<summary>🛠️ Fix failing CI checks 💡</summary>
- [ ] <!-- {"checkboxId": "6d21cfe8-ec3f-40e2-9222-b8318b64d3b0", "radioGroupId": "fix-ci-output-choice-group-unknown_comment_id"} --> Create stacked PR
- [ ] <!-- {"checkboxId": "9f0d24fb-b419-4f01-baf0-8b26b6424f34", "radioGroupId": "fix-ci-output-choice-group-unknown_comment_id"} --> Commit on current branch
</details>
<details>
<summary>🧪 Generate unit tests (beta)</summary>
- [ ] <!-- {"checkboxId": "f47ac10b-58cc-4372-a567-0e02b2c3d479", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} --> Create PR with unit tests
- [ ] <!-- {"checkboxId": "6ba7b810-9dad-11d1-80b4-00c04fd430c8", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} --> Commit unit tests in branch `new-plugins`
</details>
</details>
<!-- finishing_touch_checkbox_end -->
<!-- tips_start -->
---
Thanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=listepo/cross-code&utm_content=10)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
<details>
<summary>❤️ Share</summary>
- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai)
- [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai)
- [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai)
- [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
</details>
<sub>Comment `@coderabbitai help` to get the list of available commands.</sub>
<!-- tips_end -->
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
nx.json (1)
46-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the deprecated
servetarget unless compatibility requires it.Nx documents
serveTargetNameas deprecated anddevTargetNameas its replacement. The two names represent the same Vite development-server capability. (nx.dev)Remove
serveTargetNameunless existing scripts or projects still requirenx serve.Suggested configuration
"devTargetName": "dev", - "serveTargetName": "serve", "previewTargetName": "preview",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nx.json` around lines 46 - 50, Remove the deprecated serveTargetName entry from the Nx configuration, while retaining devTargetName as the development-server target. Preserve serveTargetName only if repository scripts or projects explicitly depend on nx serve.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nx.json`:
- Line 60: Update the preVersionCommand release hook in nx.json to invoke Nx
through the repository-pinned pnpm executable, replacing the npx command with
pnpm exec nx while preserving the existing run-many build arguments.
In `@packages/ns-wamr/package.json`:
- Around line 158-159: Update the typings.ios and typings.android scripts in
package.json to execute their respective ns typings commands from the
apps/ns-wasm-test working directory, so generated declarations are written to
that app’s typings directory.
---
Nitpick comments:
In `@nx.json`:
- Around line 46-50: Remove the deprecated serveTargetName entry from the Nx
configuration, while retaining devTargetName as the development-server target.
Preserve serveTargetName only if repository scripts or projects explicitly
depend on nx serve.
🪄 Autofix
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 94995fd9-0c17-4fed-8215-abffeff53652
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
.gitignorenx.jsonpackage.jsonpackages/ns-wamr/package.jsonpnpm-workspace.yaml
Contributor
Fixes Applied SuccessfullyFixed 2 file(s) based on 2 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 2 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Replaces the hello-world page with a live gallery of every
@nativescript/core UI widget: text, inputs, pickers, images,
HtmlView/WebView, ListView/Repeater, Placeholder, all layouts,
gestures, animations, dialogs, and Frame/Page/TabView navigation.
The app root is now a RootLayout wrapping the Frame so the popup
and shade-cover demo exercises the real API.
Verified on the iOS simulator. Four core quirks worked around:
- {{ $value }} over a primitive array is a boxed String, which
colour-typed properties cannot parse — item templates bind named
object properties instead
- TabView is controller-backed on iOS and fills the whole Page, so
it is the page's only content and Close lives inside a tab
- @nativescript/theme sets text-transform:capitalize on TabView and
it inherits into tab content — overridden on .gallery
- the template logo.png is white on transparent, so the Image demos
sit on a dark backdrop
SplitView is documented rather than demoed: at compact width the
collapsing UISplitViewController loops re-pushing nav controllers,
so it cannot be hosted in a Frame-pushed page.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/rspack-test-app/nativescript.config.ts (1)
3-11: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSet the NativeScript CLI package manager explicitly.
If this app can be run through NativeScript CLI, add
cli.packageManager: 'pnpm'. The repository-localapps/ns-wasm-test/nativescript.config.tsuses this setting. NativeScript otherwise uses the CLI-wide default, ornpmwhen no default is set. A different default can produce a dependency tree that differs from CI. (docs.nativescript.org)Suggested configuration change
appResourcesPath: 'App_Resources', + cli: { + packageManager: 'pnpm' + }, android: {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/rspack-test-app/nativescript.config.ts` around lines 3 - 11, Update the exported NativeScript configuration object to include a cli.packageManager setting of 'pnpm', matching the existing configuration pattern in the repository and ensuring CLI installs use pnpm.apps/rspack-test-app/App_Resources/Android/src/main/AndroidManifest.xml (1)
11-12: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRemove legacy storage permissions when no storage feature requires them.
The manifest declares
READ_EXTERNAL_STORAGEandWRITE_EXTERNAL_STORAGE, but the supplied app objective is a widgets gallery and no storage access is shown. If omitted app code does not require these permissions, remove them to keep the permission surface minimal.Proposed cleanup
- <uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"/> - <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/rspack-test-app/App_Resources/Android/src/main/AndroidManifest.xml` around lines 11 - 12, Remove the READ_EXTERNAL_STORAGE and WRITE_EXTERNAL_STORAGE declarations from the Android manifest, provided no app code requires storage access. Keep the manifest focused on permissions needed by the widgets gallery.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/rspack-test-app/app/app.css`:
- Around line 25-27: Update the dark-mode card styles in .ns-dark .card and the
related title and hint selectors around the referenced card content to set
accessible light text colors against the `#212121` background. Preserve the
existing dark card background while ensuring both card titles and hint text meet
sufficient contrast.
In `@apps/rspack-test-app/app/tabs-page.xml`:
- Line 3: Update the Android NavigationButton in tabs-page to bind its tap
action to the existing onClose handler by adding the tap binding, while
preserving the current text and system icon attributes.
In `@apps/rspack-test-app/package.json`:
- Around line 6-15: Add the NativeScript 9.0-compatible `@nativescript/android`
package to the devDependencies in package.json, then regenerate or update the
corresponding package-lock.json entry while preserving the existing dependency
versions.
---
Nitpick comments:
In `@apps/rspack-test-app/App_Resources/Android/src/main/AndroidManifest.xml`:
- Around line 11-12: Remove the READ_EXTERNAL_STORAGE and WRITE_EXTERNAL_STORAGE
declarations from the Android manifest, provided no app code requires storage
access. Keep the manifest focused on permissions needed by the widgets gallery.
In `@apps/rspack-test-app/nativescript.config.ts`:
- Around line 3-11: Update the exported NativeScript configuration object to
include a cli.packageManager setting of 'pnpm', matching the existing
configuration pattern in the repository and ensuring CLI installs use pnpm.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ce3704b5-c38b-45f4-b949-8f93f89f703c
⛔ Files ignored due to path filters (40)
apps/rspack-test-app/App_Resources/Android/src/main/res/drawable-hdpi/background.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/Android/src/main/res/drawable-hdpi/logo.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/Android/src/main/res/drawable-ldpi/background.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/Android/src/main/res/drawable-ldpi/logo.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/Android/src/main/res/drawable-mdpi/background.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/Android/src/main/res/drawable-mdpi/logo.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/Android/src/main/res/drawable-xhdpi/background.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/Android/src/main/res/drawable-xhdpi/logo.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/Android/src/main/res/drawable-xxhdpi/background.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/Android/src/main/res/drawable-xxhdpi/logo.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/Android/src/main/res/drawable-xxxhdpi/background.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/Android/src/main/res/drawable-xxxhdpi/logo.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/Android/src/main/res/mipmap-hdpi/ic_launcher.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/Android/src/main/res/mipmap-mdpi/ic_launcher.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/Android/src/main/res/mipmap-xhdpi/ic_launcher.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/Android/src/main/res/mipmap-xxhdpi/ic_launcher.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/Android/src/main/res/mipmap-xxxhdpi/ic_launcher.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/AppIcon.appiconset/icon-1024.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/AppIcon.appiconset/icon-20.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/AppIcon.appiconset/icon-20@2x.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/AppIcon.appiconset/icon-20@3x.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/AppIcon.appiconset/icon-29.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/AppIcon.appiconset/icon-29@2x.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/AppIcon.appiconset/icon-29@3x.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/AppIcon.appiconset/icon-40.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/AppIcon.appiconset/icon-40@2x.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/AppIcon.appiconset/icon-40@3x.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/AppIcon.appiconset/icon-60@2x.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/AppIcon.appiconset/icon-60@3x.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/AppIcon.appiconset/icon-76.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/AppIcon.appiconset/icon-76@2x.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/AppIcon.appiconset/icon-83.5@2x.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/LaunchScreen.AspectFill.imageset/LaunchScreen-AspectFill.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/LaunchScreen.AspectFill.imageset/LaunchScreen-AspectFill@2x.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/LaunchScreen.AspectFill.imageset/LaunchScreen-AspectFill@3x.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/LaunchScreen.Center.imageset/LaunchScreen-Center.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/LaunchScreen.Center.imageset/LaunchScreen-Center@2x.pngis excluded by!**/*.pngapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/LaunchScreen.Center.imageset/LaunchScreen-Center@3x.pngis excluded by!**/*.pngapps/rspack-test-app/app/images/logo.pngis excluded by!**/*.pngapps/rspack-test-app/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (35)
apps/rspack-test-app/.editorconfigapps/rspack-test-app/.gitignoreapps/rspack-test-app/.vscode/extensions.jsonapps/rspack-test-app/App_Resources/Android/app.gradleapps/rspack-test-app/App_Resources/Android/before-plugins.gradleapps/rspack-test-app/App_Resources/Android/src/main/AndroidManifest.xmlapps/rspack-test-app/App_Resources/Android/src/main/res/drawable-nodpi/splash_screen.xmlapps/rspack-test-app/App_Resources/Android/src/main/res/drawable/ic_launcher_foreground.xmlapps/rspack-test-app/App_Resources/Android/src/main/res/mipmap-anydpi-v26/ic_launcher.xmlapps/rspack-test-app/App_Resources/Android/src/main/res/values-v21/colors.xmlapps/rspack-test-app/App_Resources/Android/src/main/res/values-v21/styles.xmlapps/rspack-test-app/App_Resources/Android/src/main/res/values-v29/styles.xmlapps/rspack-test-app/App_Resources/Android/src/main/res/values/colors.xmlapps/rspack-test-app/App_Resources/Android/src/main/res/values/ic_launcher_background.xmlapps/rspack-test-app/App_Resources/Android/src/main/res/values/styles.xmlapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/AppIcon.appiconset/Contents.jsonapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/Contents.jsonapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/LaunchScreen.AspectFill.imageset/Contents.jsonapps/rspack-test-app/App_Resources/iOS/Assets.xcassets/LaunchScreen.Center.imageset/Contents.jsonapps/rspack-test-app/App_Resources/iOS/Info.plistapps/rspack-test-app/App_Resources/iOS/LaunchScreen.storyboardapps/rspack-test-app/App_Resources/iOS/build.xcconfigapps/rspack-test-app/app/app-root.xmlapps/rspack-test-app/app/app.cssapps/rspack-test-app/app/app.tsapps/rspack-test-app/app/main-page.tsapps/rspack-test-app/app/main-page.xmlapps/rspack-test-app/app/main-view-model.tsapps/rspack-test-app/app/tabs-page.tsapps/rspack-test-app/app/tabs-page.xmlapps/rspack-test-app/nativescript.config.tsapps/rspack-test-app/package.jsonapps/rspack-test-app/references.d.tsapps/rspack-test-app/tsconfig.jsonapps/rspack-test-app/webpack.config.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
packages/ns-rspack/src/lib/compat.ts (2)
184-198: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueGuard the entry-exports shim against an empty entry set.
If
entryPointsis empty,patternis an empty string. The generated test is/^()\.[cm]?js$/, which matches no real asset and still registersNsEntryExportsShim. Skip the plugin when there are no entries.♻️ Proposed guard
const entries = [...(config.entryPoints as unknown as ChainedMapLike).store.keys()] + + if (entries.length === 0) { + return config + } + const pattern = entries.map((name) => name.replace(/[.*+?^${}()|[\]\\/]/g, '\\$&')).join('|')Note: an early
returnhere would skip the laterWatchStatePluginand Terser steps. Wrap the block in anif (entries.length > 0)condition instead if you keep the flow linear.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ns-rspack/src/lib/compat.ts` around lines 184 - 198, Guard the NsEntryExportsShim registration using the entries collected from config.entryPoints in the non-outputModule path. Only create the pattern and register the BannerPlugin when entries.length is greater than zero; keep the surrounding flow linear so later WatchStatePlugin and Terser processing still executes.
68-72: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
toRspackGlobcovers only the+(...)extglob form.globby also supports
@(...),?(...),*(...), and!(...). A user copy rule that uses those forms silently copies nothing, the same failure this function exists to prevent. Extend the rewrite for@(a|b)and?(a|b), or document the limitation in the README.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ns-rspack/src/lib/compat.ts` around lines 68 - 72, The toRspackGlob function only rewrites +(…) extglobs, leaving supported @ (a|b) and ?(a|b) patterns unusable. Extend its rewrite logic to handle the requested @ and ? forms while preserving the existing +(…) conversion, or document this limitation in the README.packages/ns-rspack/src/index.ts (1)
71-76: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winA caller-supplied chain skips
adaptChain.
resolveConfigappliesadaptChainonly throughresolveChainableConfig(). If a caller builds a chain and passes it directly, the webpack-only plugins andts-loaderstay in place, and the build fails at compile time.adaptChainis effectively idempotent for an already-adapted chain, so apply it unconditionally.♻️ Proposed fix
export function resolveConfig(chainableConfig?: RspackChain): Configuration { - const chain = chainableConfig ?? resolveChainableConfig() + const chain = chainableConfig ? adaptChain(chainableConfig) : resolveChainableConfig() const resolved = nsWebpack.resolveConfig(asWebpackChain(chain))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ns-rspack/src/index.ts` around lines 71 - 76, Update resolveConfig to apply adaptChain unconditionally to the selected chain, including caller-supplied chainableConfig, before passing it to asWebpackChain and nsWebpack.resolveConfig. Preserve the existing resolveChainableConfig path while ensuring already-adapted chains remain supported.packages/ns-rspack/src/lib/watch-state-plugin.ts (1)
36-70: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCall
callback()after the hook work, not before.Both hooks signal completion first and then read state and send IPC. Two consequences follow. First, the compiler can continue past the hook while the plugin still reads
compilation.getStats(), so the reported state can belong to a compilation the compiler has already moved on from in watch mode. Second, any exception thrown aftercallback()escapes the hook and becomes an unhandled error instead of a compilation error. Move eachcallback()to the end of the hook.♻️ Proposed reordering for `afterEmit`
compiler.hooks.afterEmit.tapAsync(id, (compilation, callback) => { - callback() - if (this.options.stats) { @@ this.notify({ type: 'compilation', version: 1, hash: compilation.hash ?? undefined, data: { emittedAssets, staleAssets }, }) + + callback() })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ns-rspack/src/lib/watch-state-plugin.ts` around lines 36 - 70, Move the callback() invocation to the end of both the watchRun and afterEmit handlers, after all state updates, logging, stats processing, and notify work completes. Preserve the existing hook logic, ensuring exceptions from that work occur before signaling completion and the compiler cannot proceed until each handler finishes.packages/ns-rspack/src/lib/compat.spec.ts (1)
30-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the exact replacement plugin.
not.toBe(FakeCustomPlugin)passes for any substitution, including a wrong one. Assert the exportedContextExclusionPlugininstead.♻️ Proposed stronger assertion
-import { adaptChain, adaptConfig } from './compat.js' +import { ContextExclusionPlugin, adaptChain, adaptConfig } from './compat.js'- expect(config.plugin('ContextExclusionPlugin|App_Resources').get('plugin')).not.toBe( - FakeCustomPlugin, - ) + expect(config.plugin('ContextExclusionPlugin|App_Resources').get('plugin')).toBe( + ContextExclusionPlugin, + ) + expect(config.plugin('ContextExclusionPlugin|App_Resources').get('args')).toEqual([/x/])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ns-rspack/src/lib/compat.spec.ts` around lines 30 - 40, Update the assertion in the plugin registration test to verify that the adapted plugin is exactly the exported ContextExclusionPlugin, replacing the broad not.toBe(FakeCustomPlugin) check while preserving the existing lookup key and adaptation flow.packages/ns-rspack/src/bin/index.ts (1)
104-112: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWatch mode never closes the compiler.
compiler.watchreturns aWatchinghandle. The CLI discards it, so no code path callswatching.close(). OnSIGINTorSIGTERMthe process exits with the file watchers and the output file system still active. Keep the handle and close it on termination signals.♻️ Proposed handling
- compiler.watch(configuration.watchOptions ?? {}, (err, stats) => report(env, err, stats)) + const watching = compiler.watch(configuration.watchOptions ?? {}, (err, stats) => + report(env, err, stats), + ) + + for (const signal of ['SIGINT', 'SIGTERM'] as const) { + process.once(signal, () => { + watching.close(() => process.exit(0)) + }) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ns-rspack/src/bin/index.ts` around lines 104 - 112, Update the watch-mode branch around compiler.watch to retain its returned Watching handle and register SIGINT and SIGTERM handlers that call watching.close() before termination. Preserve the existing report callback and normal watch-mode flow, ensuring cleanup occurs only for the active watch instance.apps/rspack-test-app/nativescript.config.ts (1)
21-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNarrow the compatibility type.
bundler: stringaccepts invalid bundler names and weakens compile-time checking. Keep the NativeScript bundler type and add only'rspack'. Usesatisfiesinstead ofasso the remaining fields stay checked. The NativeScript CLI has a dedicated Rspack path; unknown values can fall through to webpack executable resolution. (github.com)This follows the supplied NativeScript CLI and package context.
Proposed type-safe declaration
-} as Omit<NativeScriptConfig, 'bundler'> & { bundler: string }; +} satisfies Omit<NativeScriptConfig, 'bundler'> & { + bundler: NativeScriptConfig['bundler'] | 'rspack'; +};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/rspack-test-app/nativescript.config.ts` at line 21, Update the NativeScript config declaration to preserve the existing NativeScript bundler type while extending it with only the literal 'rspack'. Replace the broad type assertion with satisfies so all remaining configuration fields continue to receive structural validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ns-rspack/src/bin/index.ts`:
- Around line 86-88: Update the environment setup around env.config and
process.env.NATIVESCRIPT_CONFIG_NAME so the variable is assigned only when
env.config is a string. Preserve the existing nullish assignment behavior when a
valid config name is provided, but avoid assigning when env.config is absent to
prevent the literal "undefined" value.
- Around line 27-28: Update parseEnvFlags to convert exact string values "true"
and "false" into boolean true and false while preserving all other values as
strings. Ensure the resulting env.stats value is a boolean so the existing
checks in report and the compatibility logic honor --env.stats=false.
In `@packages/ns-rspack/src/lib/compat.ts`:
- Around line 18-32: Update ContextExclusionPlugin.apply and its
contextModuleFactory hook integration so missing contextModuleFiles support
cannot silently disable exclusions: use a supported ContextModuleFactory hook
for filtering, or throw an explicit compatibility error when no supported hook
exists. Preserve negativeMatcher filtering for supported runtimes.
---
Nitpick comments:
In `@apps/rspack-test-app/nativescript.config.ts`:
- Line 21: Update the NativeScript config declaration to preserve the existing
NativeScript bundler type while extending it with only the literal 'rspack'.
Replace the broad type assertion with satisfies so all remaining configuration
fields continue to receive structural validation.
In `@packages/ns-rspack/src/bin/index.ts`:
- Around line 104-112: Update the watch-mode branch around compiler.watch to
retain its returned Watching handle and register SIGINT and SIGTERM handlers
that call watching.close() before termination. Preserve the existing report
callback and normal watch-mode flow, ensuring cleanup occurs only for the active
watch instance.
In `@packages/ns-rspack/src/index.ts`:
- Around line 71-76: Update resolveConfig to apply adaptChain unconditionally to
the selected chain, including caller-supplied chainableConfig, before passing it
to asWebpackChain and nsWebpack.resolveConfig. Preserve the existing
resolveChainableConfig path while ensuring already-adapted chains remain
supported.
In `@packages/ns-rspack/src/lib/compat.spec.ts`:
- Around line 30-40: Update the assertion in the plugin registration test to
verify that the adapted plugin is exactly the exported ContextExclusionPlugin,
replacing the broad not.toBe(FakeCustomPlugin) check while preserving the
existing lookup key and adaptation flow.
In `@packages/ns-rspack/src/lib/compat.ts`:
- Around line 184-198: Guard the NsEntryExportsShim registration using the
entries collected from config.entryPoints in the non-outputModule path. Only
create the pattern and register the BannerPlugin when entries.length is greater
than zero; keep the surrounding flow linear so later WatchStatePlugin and Terser
processing still executes.
- Around line 68-72: The toRspackGlob function only rewrites +(…) extglobs,
leaving supported @ (a|b) and ?(a|b) patterns unusable. Extend its rewrite logic
to handle the requested @ and ? forms while preserving the existing +(…)
conversion, or document this limitation in the README.
In `@packages/ns-rspack/src/lib/watch-state-plugin.ts`:
- Around line 36-70: Move the callback() invocation to the end of both the
watchRun and afterEmit handlers, after all state updates, logging, stats
processing, and notify work completes. Preserve the existing hook logic,
ensuring exceptions from that work occur before signaling completion and the
compiler cannot proceed until each handler finishes.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fb9ac7c9-fa39-4ecc-b8d5-605083909380
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
apps/rspack-test-app/nativescript.config.tsapps/rspack-test-app/package.jsonapps/rspack-test-app/rspack.config.tsapps/rspack-test-app/tsconfig.jsonpackage.jsonpackages/ns-rspack/README.mdpackages/ns-rspack/package.jsonpackages/ns-rspack/src/bin/index.tspackages/ns-rspack/src/index.tspackages/ns-rspack/src/lib/compat.spec.tspackages/ns-rspack/src/lib/compat.tspackages/ns-rspack/src/lib/watch-state-plugin.tspackages/ns-rspack/tsconfig.jsonpackages/ns-rspack/tsconfig.lib.jsonpackages/ns-rspack/tsconfig.spec.jsonpackages/ns-rspack/vitest.config.mtspnpm-workspace.yamltsconfig.json
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/rspack-test-app/package.json
- apps/rspack-test-app/tsconfig.json
- package.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ns-rspack/AGENTS.md`:
- Line 25: Update the build command reference in AGENTS.md to invoke the
workspace-local Nx through pnpm, changing the `nx run ns-rspack:build` example
to `pnpm exec nx run ns-rspack:build` while preserving the surrounding
documentation.
- Line 19: Add language identifiers to both fenced code blocks in AGENTS.md:
mark the architecture listing as text and the command block as bash or console,
preserving their existing contents.
In `@README.md`:
- Around line 233-235: Update the shared WebAssembly documentation row in
README.md to accurately reflect the newly added `@cross-code/ns-wasm-edge`,
`@cross-code/ns-wasm-chicory`, and `@cross-code/ns-endive` packages, listing all six
plugins or explicitly limiting WASM.md’s scope to the original wasm3, WAMR, and
WasmKit plugins.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c5fedc77-d393-425c-a6db-7f594ccfd420
📒 Files selected for processing (3)
AGENTS.mdREADME.mdpackages/ns-rspack/AGENTS.md
- --env.<key>=true/false now yield real booleans, matching webpack's CLI - only set NATIVESCRIPT_CONFIG_NAME when a config name was actually passed - ContextExclusionPlugin throws when the bundler exposes no contextModuleFiles hook instead of silently bundling excluded files - test app: wire NavigationButton back to onClose, dark-theme card colors
- bump TypeScript ~5.4.0 → ~6.0.3 to match the workspace (the shared tsconfig.base.json uses noUncheckedSideEffectImports, a TS ≥5.6 option) - add @nativescript/types-android/types-ios as direct devDependencies — the umbrella @nativescript/types references them via ../ paths, and pnpm only links direct deps into node_modules/@NativeScript - tsconfig: ignoreDeprecations 6.0 (baseUrl), composite + outDir/tsBuildInfoFile so the nx typecheck target (tsc --build --emitDeclarationOnly) works - root tsconfig: nx sync registers the app as a project reference
… webpack
Both apps now set bundler: 'rspack' in nativescript.config.ts and ship a
rspack.config.ts; @nativescript/webpack, ts-loader and postcss-loader are
replaced by @nativescript/rspack (file: alias to @cross-code/ns-rspack,
the name the {N} CLI resolves the bundler by). The ns-wasm-test WASM
copy rules and Vitest test entry move from webpack.config.js into
rspack.config.ts.
- vitest-ns webpack.cjs helper is now bundler-agnostic: it calls
chainRspack when the module exposes it, chainWebpack otherwise — the
chain API is identical in both
- add @nativescript/types-android/types-ios devDeps to both apps so the
umbrella @nativescript/types references resolve (pnpm only links
direct deps)
- ns-wasm-test/nativescript.config.ts type widened for bundler: 'rspack'
- project.json typecheck inputs track rspack.config.ts
- CI: build ns-rspack before the app installs (file: snapshot), and
trigger app jobs when packages/ns-rspack changes
- rspack-test-app tsconfig: nx sync's TS6 rewrite (drop baseUrl,
relative paths, ignoreDeprecations no longer needed)
The standalone apps are their own pnpm workspace roots; they now expose the sibling packages they consume through the workspace: protocol (pnpm-workspace.yaml packages: ../../packages/...), replacing file: dependencies. The apps stay separate installs with their own lockfiles — only the listed packages are reachable, never the root workspace. - ns-wasm-test: ns-wasm-core override dropped; the plugins' workspace:* core reference now resolves against the app's own workspace - ns-wry-app: ns-wry and ns-rspack linked via workspace: - CI: reword the build-before-install comments (workspace: links, no file: snapshotting) - AGENTS.md: package wiring section updated
rspack 2.x's ContextModuleFactory exposes only beforeResolve/afterResolve and enumerates context files in Rust, so the contextModuleFiles hook never exists — the previous fail-loudly behavior broke every rspack build at plugin apply time (the wasm-test app bundle failed, ns run exited 127, and vitest then timed out tearing the device worker down). The exclusion is best-effort: the ~/ context is scoped to the app dir (App_Resources is a sibling), and platform-suffixed files are rare. Warn once instead of throwing; keep the hook tap for bundlers that do expose it. Verified end-to-end: the ns-wasm-test iOS bundle compiles, emits the vitest-ns worker entry, enumerates all 8 spec files via require.context, and copies both .wasm fixtures.
The app's hooks dir had two before-checkForChanges trampolines: nativescript-core.mjs (core 9.0.20, correct) and nativescript-core.js (stale, requiring a .js cli-hook that core 9.0.20 does not ship). The CLI loads every file in the hooks dir, so the stale .js one threw 'Cannot find module @nativescript/core/cli-hooks/before-checkForChanges.js' after the first rspack compilation and ns run exited 127. The .js trampoline is written by @nativescript/core 8.9.9's postinstall (@nativescript/hook 2.0.0). It entered the app's install because the app workspace now includes packages/* as members and pnpm installs their devDependencies — ns-wamr declared a vestigial @nativescript/core ^8.9.0 devDep that nothing imports. - remove the unused @nativescript/core devDep from packages/ns-wamr - regenerate root + app lockfiles (core 8.9.9 and hook 2.0.0 gone) - delete the stale nativescript-core.js trampolines in both apps (hooks/ is gitignored; core 9.0.20's postinstall regenerates the .mjs hook) - nx sync: apps' tsconfigs gain the ns-rspack project reference - ci.yml: drop continue-on-error from wasm-test-android per the never-continue-on-error policy (AGENTS.md)
Rstest has no custom-pool API, so @cross-code/ns-rstest owns its own Node host: runNativeScriptTests() globs the spec files, launches the NativeScript CLI (ns run --env.rstestNativeScript), round-robins files across device worker slots, and reports through Rstest's Reporter interface. On device it drives Rstest's own browser runtime (@rstest/core/internal/browser-runtime) inside a Worker; a shim on globalThis['@rstest/core'] forwards the per-file API Rstest publishes, and the optional results view ships from @cross-code/ns-rstest/ui. - new package packages/ns-rstest (Node host, protocol, worker runtime, shim, UI); deletes packages/vitest-ns and packages/vitest-ns-ui - app: vitest-ns.ts/.worker.ts -> ns-rstest.ts/.worker.ts; specs import from @rstest/core; host runners are ns-rstest.ios.mts / .android.mts run via node (type stripping); rspack.config.ts swaps the bundler hook for configureNativeScriptRstest (env.rstestNativeScript, Istanbul coverage) - nx-ns-app: drop the vitest pool option from NsTestOptions - CI/docs: vitest-ns references -> ns-rstest; device jobs build ns-rstest before installing the app - crlf -> lf normalization in ci.yml / code-review.yml Verified: 16-project test+typecheck+build all pass; the ns-rstest host launches the CLI, rspack emits the worker bundle with all 8 spec files enumerated, and the rstest entry swaps in under env.rstestNativeScript.
Adds @cross-code/ns-lynx, a NativeScript plugin embedding the Lynx engine as
a <LynxView> inside a NativeScript host page. NativeScript is the host; Lynx
renders a ReactLynx bundle subtree. The plugin has no native layer: Lynx
ships its engine via CocoaPods (iOS) and Maven Central (Android), and the
metadata/static-binding generators expose those classes to JS directly.
- src/lib/bundle.ts — src→location rules + JSON coercion (unit-tested)
- lynx-view-{common,ios,android}.ts — properties/events, File.readSync bytes
handed to LynxLoadMeta.binaryData (no TemplateProvider subclass)
- platforms/ios/Podfile + platforms/android/include.gradle — pinned Lynx 4.0.1
/ PrimJS 4.0.0; Demote iOS warnings-as-errors and drop PrimJS's module map
so the metadata generator stops crashing on its C++ headers
- apps/ns-lynx-app — worked example: rspeedy ReactLynx bundle copied into the
app folder by rspack, rendered in a <LynxView> beside native views
- CI: build + coverage ns-lynx; docs in root AGENTS/README
Replace the @nativescript/webpack compat layer with a first-party rspack pipeline, prove it on ns-lynx-app via ns-rstest, and prefix test entries so they stay out of require.context. Co-authored-by: Cursor <cursoragent@cursor.com>
native-class-strip-loader only matched `class` and `export class`, so
`@NativeClass export default class X extends NSObject` reached the device
as an ES6 class and threw when the runtime constructed it. Extend the
lookahead to `export default` and `abstract` in every valid combination.
getIPS() returned loopback addresses alongside real ones, so
`__NS_DEV_HOST_IPS__` told the device to look for the dev host at
127.0.0.1 — itself. Skip internal interfaces.
Dev builds now always inline source maps, for the reason the comment
already gave: Chrome DevTools cannot fetch external .map files in this
debugging flow, so the external maps {N} 9 used to get were unusable.
Also keep the test-only project fixture out of the published build, and
stop claiming there is no webpack in the dependency tree — webpack-merge
is still there as a deep-merge helper.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…g them `ns typings` output is generated from the built platform project, but the root ignore rule was `/typings` — anchored to the repo root, so it only covered a directory that never existed there. Running `ns typings android` from a plugin package left a stray `typings/` behind in ns-wamr, ns-wasm3 and ns-wasm-chicory, permanently visible as untracked noise. Ignore `typings/` at any depth and drop the three strays. Give the generation a home instead: ns-wasm-test owns `platforms/`, so it owns the Nx targets, which depend on `prepare.<platform>` and declare `typings/<platform>` as cacheable output. ns-wamr's package scripts now delegate there rather than repeating the command with a relative `cd` — the shape that produced the strays in the first place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
webpack-merge was the last thing keeping "webpack" in this package's
dependency tree, and it was only ever there for one function. rspack-merge
is the replacement rspack's own docs name: a zero-dependency fork with an
identical signature, published under the same org as rspack-chain, which
this package already depends on. The swap is an import change — `merge`,
`Utils.merge` and the plugin-facing `api.merge` keep working unchanged for
third-party configs.
`merge` is public API, so its semantics are now pinned by tests rather than
by whatever the dependency happens to do: plugins concatenate as live
instances instead of being deep-merged, RegExp rule tests survive, nested
splitChunks cacheGroups deep-merge, function values stay callable, and the
inputs are not mutated.
Tests move from vitest to Rstest, rspack's own runner. Two things that
migration forces, both of which are properties of Rstest bundling sources
through rspack rather than transforming them per file:
- hmr-runtime.ts now reads `module` through a local binding. A direct
`module.hot` test is statically folded to a constant and the branch
deleted — and that function is stringified, not executed, so the
bundler was silently emitting an empty HMR runtime.
- the module-mock APIs are rewritten by a native rspack plugin and must
be called literally as `rs.mock()` / `rs.resetModules()`, so the specs
rename the identifier rather than aliasing the import.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Istanbul instruments the source, so the numbers do not depend on the engine
running the tests, and it is what the device suites in @cross-code/ns-rstest
already have to use — the {N} runtimes expose no V8 coverage. Both halves of
the repo now report the same way. Rstest offers no third provider.
Switching provider surfaced a latent hazard rather than just changing a
number: hmr-runtime.ts exports a function that is stringified and appended
to the app bundle, never called. Istanbul instrumented it, so the text
carried `cov_*()` counters referencing a global that does not exist on
device, and two specs failed with a ReferenceError. An `istanbul ignore
next` keeps the function reaching `.toString()` untouched — the same
constraint that already forces it to read `module` through a local binding.
Also ignore `test-output/` at any depth: the coverage report lands there,
and the previous rule was pinned to apps/ns-wasm-test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Move NSArray/Java byte helpers into ns-wasm-core and replace adapter `any` with structural native-api types so wrong selectors fail at compile time. Fix wamr-ffi call_raw slot sizing and add regression tests for multi-return calls. Co-authored-by: Cursor <cursoragent@cursor.com>
Add durable agent gotchas at the repo and ns-rspack package roots, split ns-lynx-app production vs rstest tsconfig like ns-wasm-test, run nx-ns-app executors from the app directory, and fix rstest results double-rendering. Co-authored-by: Cursor <cursoragent@cursor.com>
Import WamrError from wire.js so toThrow assertions match at runtime, drop spec `any`, and link tsconfig.spec.json to the library project for build mode. Co-authored-by: Cursor <cursoragent@cursor.com>
Apply the same structural native-api declarations and ns-wasm-core bridge helpers as wasm3/wamr, and tighten the device-mirror fakes so typecheck passes. Co-authored-by: Cursor <cursoragent@cursor.com>
…switch Update ns-wasm-test and ns-wry-app pnpm-lock.yaml so frozen-lockfile CI installs match @rstest/coverage-istanbul in packages/ns-rspack/package.json. Co-authored-by: Cursor <cursoragent@cursor.com>
Add tools/verify-nested-lockfiles.mjs and run it in unit-tests so stale apps/*/pnpm-lock.yaml files fail fast when linked packages like ns-rspack change dependencies without regenerating each nested lockfile. Co-authored-by: Cursor <cursoragent@cursor.com>
The lockfile verification commit accidentally staged unrelated deletions. Restore the ambient native declarations both plugins still reference. Co-authored-by: Cursor <cursoragent@cursor.com>
…-edge, wry Same pattern already applied to wamr/wasm3/wasm-kit: replace `any`-typed globalThis probes with ambient declarations scoped to the surface each adapter actually calls, so a renamed method or wrong arity fails the build instead of returning `undefined` on a device. ns-wasm-core's nativeArrayToJs now also handles java.util.List's size()-based length (iOS NativeArrayLike only had .count/.length), which is what lets ns-endive drop its local copy of the same list-walking loop. .oxlintrc.json un-ignores hand-written src/**/*.d.ts — it was blanket- ignoring every .d.ts, generated or not, so oxlint's no-explicit-any rule never actually reached these ambient declarations. ns-wry's Rust crates get the same #![deny(unsafe_op_in_unsafe_fn)] already applied to the other native crates, verified via `cargo check`. Also cleans up the remaining `any` in the two test apps and in nx-ns-app's executor specs, and wires nx-ns-app's tsconfig.spec.json into typecheck so the spec files stay covered going forward. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
On edition 2021, a temporary created in an `if let` scrutinee lives to the end of the whole `if let` block, not just the condition — so `lock(&GLOBAL_LAST_RESULTS)`'s MutexGuard stayed alive through the entire body, and the body's own `*lock(&GLOBAL_LAST_RESULTS) = None` re-lock deadlocked against it. `cargo test -p wamr-sys` hung indefinitely at 0% CPU. Read the handle out and drop the guard before branching on it, same as the call sites already do elsewhere in this file. Verified with `cargo test -p wamr-sys --test integration_test` (bounded to 90s given the prior hang): 7/7 pass in under a millisecond. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Five iOS bridge methods were declared and called under the wrong flattened @objc selector: the first, unlabeled Swift parameter was mistakenly folded into the name (e.g. loadModuleBytesError instead of loadModuleFromBytesError). An unlabeled Swift parameter contributes nothing to the selector, so these calls resolved to methods the real NSWasmKit runtime never exposed, failing every device test. Corrected linkHostFunctionModuleNameNameSignatureCallbackError, getGlobalNameError, setGlobalNameValueError, loadModuleBytesError, and loadModuleFileError to match the actual Swift signatures.
NSCWamrHostFunction/NSCWasm3HostFunction are Kotlin fun interfaces
(single-abstract-method), not classes. The adapters called them
through NativeScript's .extend() subclassing hook, which only applies
to real subclassable classes; against a fun interface it fails
resolving a nonexistent 0-arg super-constructor on every device test.
Construct the interface directly with an object exposing `invoke`,
matching the working pattern already used for ns-wasm-chicory's
NSCChicoryHostFunction. Retype NSCWa{mr,sm3}HostFunctionClass from
.extend() to a constructor signature, and drop the now-unreachable
static extend() shim from the three Vitest fakes.
Whitespace-only rustfmt reflow to fix the failing fmt.rust CI check for ns-wamr, ns-wasm3, ns-wry. No behavior change.
Every host-import call on-device failed with "host function returned invalid values" once the prior SAM-constructor fix let the callback actually run: linkHostFunction boxed the result as java.lang.Double before returning it from the invoke() callback, but that boxing is only correct for values passed *into* a Java method call. A value returned *from* a JS callback needs to cross the NativeScript bridge as a plain JS type so the bridge can convert it itself — mirroring ns-wasm-chicory's makeAndroidHostCallback, which already documents this exact distinction. Not caught locally: the Vitest fakes invoke the callback directly in JS with no native marshaling involved, so this class of bug only surfaces on a real device.
nativeArrayToJs only knew how to read an NSArray (.count/.objectAtIndex)
or a java.util.List (.size()/.get()). A host-import's arguments cross
the NativeScript Android bridge as neither: a plain-JS-like wrapper
with .length and indexed access only. Every element silently read as
undefined, normalized to the literal string "undefined", so every
host function on Android ran on garbage input — the same trap
("host function returned invalid values") the prior return-boxing fix
addressed, but from the argument side, which is why wamr/wasm3's
fixture-module host-function tests kept failing after that fix.
ns-wasm-chicory already worked around this with its own local
javaArrayToJs; folded the same fallback into the shared helper so
wamr and wasm3 (and any future adapter) get it too, and added
native-bridge.spec.ts to cover the three known array shapes directly
— this class of bug has repeatedly only surfaced on-device.
- Remove prebuilt iOS XCFrameworks, Android AARs, and native symbols from Git - Add .gitignore patterns for native build outputs and SwiftPM caches - Update AGENTS.md with the new nx-buck2/Buck2 build workflow - Update CI to install Buck2 and build artifacts on demand - Add nx-buck2 executor support for debug/release profiles - Update BUCK files, package.json files, and Rust build configs
Migrate nx-buck2 compiled output to dist/ via @nx/js:tsc and pin RUSTUP_HOME/CARGO_HOME when buck2 overrides HOME so cargo-ndk finds the toolchain. Redirect Gradle build dirs outside platforms/android/, fix jniLibs paths to follow the redirect, add gradlew-retry for transient genrule failures, and correct the CI emulator script's per-line shell constraints. Co-authored-by: Cursor <cursoragent@cursor.com>
Owner
Author
|
@coderabbitai full review |
Contributor
|
Owner
Author
|
@coderabbitai review |
Contributor
|
Buck2 genrules write nativescript-*.aar into the srcs sandbox, so CI built the app without the engine AARs and device tests failed with native runtime not found. Install the artifact next to include.gradle and fail fast if it is missing. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Tests