add activitySelectionMetadata and set functions#45
Conversation
WalkthroughThis pull request updates CI workflows to replace Node.js with Bun, refines directory paths and linting configurations, and reorganizes the project structure. In the mobile app, legacy shield components are replaced by new UI components for set operations and whitelisting, along with updated navigation. Significant enhancements are made to the native iOS module for device activity selection through new set operation functions, type definitions, and view components. Several test suites and configuration files have been added, removed, or modified to support these changes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SetOpsTab
participant ActivityPicker
participant DeviceActivityModule
User->>SetOpsTab: Request family activity selection (Family A)
SetOpsTab->>ActivityPicker: Open selection view
ActivityPicker-->>SetOpsTab: Return Family A selection
SetOpsTab->>DeviceActivityModule: Compute set operations (intersection, union, etc.)
DeviceActivityModule-->>SetOpsTab: Return computed results
Note over SetOpsTab: UI updated with set operation results
sequenceDiagram
participant ExternalTrigger
participant DeviceActivityMonitorExtension
participant ReactNativeDeviceActivityModule
ExternalTrigger->>DeviceActivityMonitorExtension: executeAction(action, placeholders, eventKey)
DeviceActivityMonitorExtension->>ReactNativeDeviceActivityModule: Process action (e.g., blockSelection, enableBlockAllMode)
ReactNativeDeviceActivityModule-->>DeviceActivityMonitorExtension: Return action result or error
DeviceActivityMonitorExtension-->>ExternalTrigger: Action execution completed
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
ios/ReactNativeDeviceActivityModule.swift(2 hunks)src/ReactNativeDeviceActivity.types.ts(2 hunks)src/ReactNativeDeviceActivityModule.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Swift Test (example project)
- GitHub Check: Expo Bundle (example project)
🔇 Additional comments (7)
src/ReactNativeDeviceActivityModule.ts (2)
26-34: LGTM!The function correctly implements a mock for iOS-specific functionality, providing appropriate default values and warning messages.
46-50: LGTM!The mock module correctly implements all the new set operation functions and metadata retrieval using the warning function.
src/ReactNativeDeviceActivity.types.ts (3)
25-30: LGTM!The type definition correctly captures all required metadata properties with appropriate types.
32-32: LGTM!The type is correctly updated to use the new
ActivitySelectionWithMetadatatype.
251-269: LGTM!The method signatures are correctly defined with appropriate parameter types and return types for all set operations and metadata retrieval.
ios/ReactNativeDeviceActivityModule.swift (2)
58-63: LGTM!The struct definition correctly captures all required metadata properties with appropriate Swift types.
630-643: LGTM!The function correctly processes the activity selection and returns appropriate metadata.
There was a problem hiding this comment.
Actionable comments posted: 36
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (34)
apps/example/android/app/src/main/res/drawable-hdpi/splashscreen_image.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/drawable-mdpi/splashscreen_image.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/drawable-xhdpi/splashscreen_image.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/drawable-xxhdpi/splashscreen_image.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/drawable-xxxhdpi/splashscreen_image.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/drawable/kingstinct.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/mipmap-hdpi/ic_launcher.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/mipmap-hdpi/ic_launcher_foreground.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/mipmap-hdpi/ic_launcher_round.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/mipmap-mdpi/ic_launcher.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/mipmap-mdpi/ic_launcher_foreground.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/mipmap-mdpi/ic_launcher_round.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/mipmap-xhdpi/ic_launcher.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/mipmap-xhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/mipmap-xhdpi/ic_launcher_round.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/mipmap-xxhdpi/ic_launcher.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/mipmap-xxhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/mipmap-xxhdpi/ic_launcher_round.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/mipmap-xxxhdpi/ic_launcher.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/mipmap-xxxhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngapps/example/android/app/src/main/res/mipmap-xxxhdpi/ic_launcher_round.pngis excluded by!**/*.pngapps/example/android/gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jarapps/example/assets/adaptive-icon.pngis excluded by!**/*.pngapps/example/assets/favicon.pngis excluded by!**/*.pngapps/example/assets/icon.pngis excluded by!**/*.pngapps/example/assets/kingstinct.pngis excluded by!**/*.pngapps/example/assets/splash.pngis excluded by!**/*.pngapps/example/ios/Podfile.lockis excluded by!**/*.lockapps/example/ios/reactnativedeviceactivityexample/Images.xcassets/AppIcon.appiconset/App-Icon-1024x1024@1x.pngis excluded by!**/*.pngapps/example/ios/reactnativedeviceactivityexample/Images.xcassets/SplashScreen.imageset/image.pngis excluded by!**/*.pngapps/example/ios/reactnativedeviceactivityexample/Images.xcassets/SplashScreenBackground.imageset/image.pngis excluded by!**/*.pngapps/example/ios/reactnativedeviceactivityexample/Images.xcassets/kingstinct.imageset/kingstinct.pngis excluded by!**/*.pngbun.lockis excluded by!**/*.lockexample/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
.github/workflows/pr.yml(1 hunks).github/workflows/test.yml(6 hunks).husky/pre-commit(1 hunks).swiftlint.yml(1 hunks)apps/example/Monitoring.tsx(2 hunks)apps/example/app.json(2 hunks)apps/example/components/ActivityPicker.tsx(1 hunks)apps/example/index.js(1 hunks)apps/example/ios/reactnativedeviceactivityexample.xcodeproj/project.pbxproj(3 hunks)apps/example/metro.config.js(1 hunks)apps/example/package.json(3 hunks)apps/example/screens/AllTheThings.tsx(6 hunks)apps/example/screens/ShieldTab.tsx(1 hunks)apps/example/tsconfig.json(1 hunks)apps/example/webpack.config.js(1 hunks)example/metro.config.js(0 hunks)example/screens/ShieldTab.tsx(0 hunks)example/tsconfig.json(0 hunks)example/webpack.config.js(0 hunks)expo-module.config.json(0 hunks)package.json(2 hunks)packages/react-native-device-activity/app.plugin.js(1 hunks)packages/react-native-device-activity/config-plugin/getAppGroupFromExpoConfig.js(1 hunks)packages/react-native-device-activity/config-plugin/withCopyTargetFolder.js(2 hunks)packages/react-native-device-activity/expo-module.config.json(1 hunks)packages/react-native-device-activity/ios/ReactNativeDeviceActivityModule.swift(6 hunks)packages/react-native-device-activity/ios/ReactNativeDeviceActivityView.swift(1 hunks)packages/react-native-device-activity/ios/ReactNativeDeviceActivityViewPersisted.swift(1 hunks)packages/react-native-device-activity/ios/ScreenTimeActivityPicker.swift(1 hunks)packages/react-native-device-activity/ios/Shared.swift(12 hunks)packages/react-native-device-activity/package.json(1 hunks)packages/react-native-device-activity/src/DeviceActivitySelectionViewPersisted.ios.tsx(1 hunks)packages/react-native-device-activity/src/DeviceActivitySelectionViewPersisted.tsx(1 hunks)packages/react-native-device-activity/src/ReactNativeDeviceActivity.types.ts(6 hunks)packages/react-native-device-activity/src/ReactNativeDeviceActivityModule.ts(2 hunks)packages/react-native-device-activity/src/index.ts(5 hunks)packages/react-native-device-activity/targets/ActivityMonitorExtension/DeviceActivityMonitorExtension.swift(1 hunks)packages/react-native-device-activity/targets/ShieldAction/ShieldActionExtension.swift(3 hunks)packages/react-native-device-activity/targets/ShieldConfiguration/ShieldConfigurationExtension.swift(7 hunks)
💤 Files with no reviewable changes (5)
- example/tsconfig.json
- example/metro.config.js
- example/webpack.config.js
- expo-module.config.json
- example/screens/ShieldTab.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
apps/example/webpack.config.js
[error] 3-3: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
apps/example/metro.config.js
[error] 4-4: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
apps/example/screens/ShieldTab.tsx
[error] 296-296: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 297-297: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 298-298: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 300-301: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 304-305: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
packages/react-native-device-activity/src/index.ts
[error] 362-362: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 367-367: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
🪛 YAMLlint (1.35.1)
.github/workflows/test.yml
[error] 119-119: trailing spaces
(trailing-spaces)
[error] 158-158: trailing spaces
(trailing-spaces)
[error] 161-161: trailing spaces
(trailing-spaces)
.github/workflows/pr.yml
[error] 27-27: no new line character at the end of file
(new-line-at-end-of-file)
.swiftlint.yml
[error] 14-14: wrong indentation: expected 2 but found 0
(indentation)
[error] 16-16: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Swift Test (example project)
- GitHub Check: SwiftLint (example project)
🔇 Additional comments (84)
packages/react-native-device-activity/expo-module.config.json (1)
1-6: Expo module configuration is properly structured.This configuration correctly defines the iOS platform support and specifies the two modules that are part of the package. The JSON is well-formatted and provides clear information about module integration.
apps/example/tsconfig.json (1)
1-11: Well-configured TypeScript setup with proper path aliases.The TypeScript configuration properly extends Expo's base configuration and includes important compiler options:
- Enabling strict type checking is a good practice for catching potential type errors
- Custom path mappings for the react-native-device-activity module will simplify imports
- The configuration properly handles both direct module imports and submodule imports
This configuration will improve development experience and code maintainability.
apps/example/index.js (1)
1-5: Standard Expo entry point implementation.This file correctly:
- Imports the necessary
registerRootComponentfunction from Expo- Imports the main App component from the local file
- Registers the App component as the root component for the application
This follows the recommended pattern for Expo applications and serves as a proper entry point.
packages/react-native-device-activity/config-plugin/withCopyTargetFolder.js (1)
11-11: Remove empty line.This is just a blank line that was added - no impact on functionality.
.husky/pre-commit (1)
1-2:Details
❓ Verification inconclusive
Verify that both paths align with the project structure.
The paths have been updated to reflect the new project structure. Ensure that all Swift files in the specified directories will be processed by the format and lint commands.
🏁 Script executed:
#!/bin/bash # Verify that the specified directories exist in the project echo "Checking if the Swift directories exist..." ls -la ./packages/react-native-device-activity/ios ls -la ./packages/react-native-device-activity/targets ls -la ./apps/example/ios echo "Checking if the SwiftLint binary exists at the specified path..." ls -la apps/example/ios/Pods/SwiftLint/swiftlintLength of output: 2191
Action Required: Verify SwiftLint Binary Path
- The directory paths used for the Swift formatting command (i.e., for the
react-native-device-activityandexample/iosdirectories) correctly reflect the current project structure.- However, the specified SwiftLint binary at
apps/example/ios/Pods/SwiftLint/swiftlintwas not found. Please verify if:
- SwiftLint is installed through CocoaPods in a different path, or
- The command should point to a globally installed binary or another location.
- Update the pre-commit script accordingly to ensure that all Swift files are linted as intended.
apps/example/app.json (1)
37-37: Bracket formatting aligned with surrounding code style.The closing bracket has been moved to the same line as the closing curly brace, which aligns with the formatting style used throughout the file.
packages/react-native-device-activity/package.json (1)
1-54:Details
❓ Verification inconclusive
Verify package version and name consistency
The package version is set to "0.4.13", but the PR title suggests this is adding new functionality. Consider whether this warrants a version bump following semantic versioning principles, especially if introducing new ActivitySelectionWithMetadata features.
Also, the AI summary mentioned this package might be renamed to "react-native-device-activity-mono", but it's still "react-native-device-activity" here. Please confirm this is intentional.
🏁 Script executed:
#!/bin/bash # Check for references to package name in the codebase echo "Checking for references to package name..." rg -l "react-native-device-activity-mono" || echo "No references to react-native-device-activity-mono found" # Check if this is a version bump from a previous version echo "Checking git history for previous package version..." git log -p -- packages/react-native-device-activity/package.json | grep -A 2 -B 2 "version" | head -n 10 || echo "No previous version found in git history"Length of output: 264
Action Required: Confirm Version Bump and Package Naming Consistency
- The package version remains "0.4.13" even though the PR introduces new functionality (e.g., ActivitySelectionWithMetadata). Please verify if this change requires a semantic version bump.
- A repository-wide search returned a reference to "react-native-device-activity-mono" (in at least one "package.json"), yet the package name here is "react-native-device-activity". Confirm that this naming decision is intentional or update accordingly.
apps/example/Monitoring.tsx (1)
10-13: Good use of named imports for better code organizationUsing named imports from the library improves code readability and allows better IDE autocompletion. Consider using only named imports throughout the file for consistency.
apps/example/metro.config.js (2)
1-42: Well-structured Metro configuration for monorepo workspacesThe Metro configuration is well-structured for a monorepo workspace, properly resolving modules from both the project and workspace root. The configuration to explicitly include the react-native-device-activity package in watchFolders and extraNodeModules ensures Metro will properly handle the package during development.
🧰 Tools
🪛 Biome (1.9.4)
[error] 4-4: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
4-4: 🧹 Nitpick (assertive)✅ Verification successful
Use Node.js import protocol for built-in modules
Use the node: protocol when importing Node.js built-in modules for better clarity and to follow modern best practices.
- const path = require("path"); + const path = require("node:path");
🌐 Web query:
Why should I use node: protocol for Node.js built-in modules?💡 Result:
Using the
node:protocol for Node.js built-in modules offers key advantages in code clarity and reliability:1. Explicit Module Identification
- The
node:prefix clearly distinguishes built-in modules from third-party or local modules. For example:This eliminates ambiguity, especially for developers unfamiliar with Node.js's core modules[1][3][6].import fs from 'node:fs/promises'; // Explicit built-in import lodash from 'lodash'; // Third-party module
2. Prevents Module Overrides
- Without
node:, a malicious or accidentalnode_modulespackage namedfscould override the built-infsmodule. The protocol ensures Node.js prioritizes its native modules[1][3][6].
3. Tooling and Ecosystem Support
- Linting: Tools like
eslint-plugin-unicornand Biome enforcenode:usage for built-ins via rules likeprefer-node-protocol[8][10].- TypeScript: Supported via
@types/node(v16+)[1][6].
Compatibility
Node.js Version Support Scope ≥ v16.0.0 Full (ESM + CommonJS) ≥ v14.18.0 Full (ESM + CommonJS) ≥ v12.20.0 ESM only
Practical Usage
CommonJS
const fs = require('node:fs'); // Preferred const crypto = require('crypto'); // Legacy (risks override)ES Modules
import { readFile } from 'node:fs/promises'; // Explicit import { readFile } from 'fs/promises'; // Implicit (avoid)By adopting
node:, developers reduce ambiguity, enhance security, and align with modern tooling standards[1][3][8].Citations:
- 1: https://2ality.com/2021/12/node-protocol-imports.html
- 2: https://nodejs.org/api/modules.html
- 3: https://www.wisdomgeek.com/development/web-development/javascript/node-js-introduces-node-protocol-imports/
- 4: https://www.w3schools.com/nodejs/ref_modules.asp
- 5: https://www.youtube.com/watch?v=h-VMZ6DqSYw
- 6: https://dev.to/yenyih/new-nodejs-protocol-imports-3pnf
- 7: https://www.jonathancreamer.com/write-async-node-scripts-using-the-only-native-nodejs-modules/
- 8: https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/prefer-node-protocol.md
- 9: https://joyeecheung.github.io/blog/2021/07/06/how-does-node-js-load-its-builtins-native-modules/
- 10: https://biomejs.dev/linter/rules/use-nodejs-import-protocol/
Refactor: Use
node:protocol for built-in modules inapps/example/metro.config.jsThe updated code should explicitly use the Node.js import protocol. This change clarifies that the module comes from Node.js built-ins and protects against potential module overrides. Based on the research, using the
node:prefix improves code clarity, aligns with modern best practices, and supports tooling such as ESLint plugins and TypeScript definitions.
- Location:
apps/example/metro.config.js, line 4- Proposed Diff:
- const path = require("path"); + const path = require("node:path");🧰 Tools
🪛 Biome (1.9.4)
[error] 4-4: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
packages/react-native-device-activity/ios/ReactNativeDeviceActivityView.swift (3)
14-14: Replace ScreenTimeSelectAppsContentView with ActivityPicker typeGood update to change the contentView type to use the new ActivityPicker component. This alignment ensures type safety.
20-20: Initialize contentView with ActivityPickerThe initialization has been correctly updated to use ActivityPicker instead of ScreenTimeSelectAppsContentView, maintaining the same model parameter.
59-64:Details
✅ Verification successful
Ensure ActivitySelectionWithMetadata structure matches TypeScript definition
The event data structure being passed to onSelectionChange includes application, category, and web domain counts. Verify that the
ActivitySelectionWithMetadataTypeScript interface in the React Native code matches this structure exactly, especially the property names and types.
🏁 Script executed:
#!/bin/bash # Check the TypeScript interface definition echo "Checking TypeScript interface for ActivitySelectionWithMetadata..." rg -A 10 "export (type|interface) ActivitySelectionWithMetadata" packages/react-native-device-activity/Length of output: 1492
Verified TypeScript-Swift Structure Alignment
The TypeScript definition for
ActivitySelectionWithMetadata(found inpackages/react-native-device-activity/src/ReactNativeDeviceActivity.types.ts) combines thefamilyActivitySelectionproperty with the additional metadata (fromActivitySelectionMetadata) that is expected to include theapplicationCount,categoryCount, andwebDomainCountproperties. The Swift event payload sent viaonSelectionChangeuses exactly these keys. No further action is required..github/workflows/test.yml (2)
51-51: Working directory correctly updated to match new project structureThe update to use
apps/exampleas the working directory for the typecheck command aligns with the project's reorganization, ensuring the command executes in the correct context.
66-66: Working directories properly updated for Expo commandsThe working directory paths for Expo commands (export, config, prebuild) have been consistently updated to match the new project structure with the app now residing in the
apps/exampledirectory.Also applies to: 81-81, 96-96
apps/example/components/ActivityPicker.tsx (4)
1-8: Import structure looks goodThe imports are well organized, clearly separating React Native core components, the device activity library components, and the UI library imports.
28-47: Props interface is well definedThe props for ActivityPicker are clearly typed with appropriate use of TypeScript for the event structure and optionality.
86-141: ActivityPickerPersisted implements enhanced functionalityThe persisted version of the picker correctly uses the ActivitySelectionMetadata type for the event and adds support for the includeEntireCategory option, which aligns with the changes in the Swift code.
The same style duplication issue exists here as mentioned in the previous comment about the ActivityPicker component.
63-79:Details
❓ Verification inconclusive
Verify the pointerEvents setting
The
pointerEvents: "none"property on the DeviceActivitySelectionView will prevent user interaction with this component. Ensure this is intentional and doesn't conflict with the selection functionality.If users are expected to interact with the selection view, you should remove this property. If there's another interaction mechanism at work, consider adding a comment explaining why interactions are disabled.
🏁 Script executed:
#!/bin/bash # Search for other instances of DeviceActivitySelectionView to understand the interaction pattern rg -A 5 "DeviceActivitySelectionView" --type tsx --type tsLength of output: 86
I'm noticing that the previous search command failed because the "tsx" file type wasn’t recognized. To gather more context about how
DeviceActivitySelectionViewis used (and whether disabling pointer events is intentional), please run a new search that adds support for TSX files. For example, you could try:Once you’ve confirmed whether this component ever appears with a different
pointerEventssetting or has any inline comments explaining why interactions are disabled, please verify manually that this behavior is intentional.
Action Required: Verify the Disabled Interactions
• Confirm thatpointerEvents: "none"on the<DeviceActivitySelectionView>inapps/example/components/ActivityPicker.tsx(lines 63–79) is intended for the final UI behavior.
• If user interaction with this component is expected, remove or adjust the property and add a corresponding comment. Otherwise, ensure the current implementation won’t cause unexpected issues.
🏁 Script executed:
#!/bin/bash # Search for references to DeviceActivitySelectionView across TSX files by adding a TSX file type rg "DeviceActivitySelectionView" --type-add 'tsx:*.tsx' --type tsx .Length of output: 3030
Action Required: Confirm Intentional Disabled Interactivity in DeviceActivitySelectionView
- In
apps/example/components/ActivityPicker.tsx(lines 63–79), the<DeviceActivitySelectionView>is rendered with the style propertypointerEvents: "none", which completely disables user interaction with it.- Our search across the codebase shows multiple references to
DeviceActivitySelectionView(e.g., in Monitoring and CreateActivity components) without any accompanying comments or variations in thepointerEventssetting.- Please verify that disabling pointer events is intentional. If user interaction is meant to be enabled in this context, remove or modify the property and add an inline comment explaining the change. If the current behavior is intended, consider adding a comment clarifying why interactions are disabled to avoid confusion in the future.
packages/react-native-device-activity/ios/ScreenTimeActivityPicker.swift (3)
21-24: New properties added for enhanced selection controlThe addition of
activitySelectionIdandincludeEntireCategoryproperties to the model is a good enhancement that provides more flexibility in how activities are selected and persisted.
30-30: Renamed struct for better clarityRenaming from
PickertoActivityPickerimproves the specificity and makes the purpose of this struct clearer. This is a good change that follows Swift naming conventions.
36-37: Simplified property accessSimplifying the access of
headerTextandfooterTextfrom$model.headerText.wrappedValuetomodel.headerTextmakes the code cleaner and more readable.apps/example/package.json (4)
10-10: New bundling script addedA new script for bundling the application has been added, which is useful for generating deployable builds. This is a good addition.
24-24: Updated dependency reference to use workspaceChanging the dependency reference from a relative path to a workspace reference is appropriate for a monorepo structure. This aligns with the project reorganization.
35-35: Updated native modules directory pathThe
nativeModulesDirpath has been adjusted to reflect the new project structure, which is necessary for proper auto-linking.
4-4:Details
✅ Verification successful
Main entry point updated
The main entry point has been changed from "expo/AppEntry.js" to "index.js". This change should align with the project's file structure.
Make sure that the index.js file exists and properly initializes the application:
🏁 Script executed:
#!/bin/bash # Check if index.js exists in the apps/example directory ls -l apps/example/index.js # Check the content of the index.js file to ensure it's properly initializing the app cat apps/example/index.jsLength of output: 228
Main entry point verified and updated
The change to using
"index.js"in the package.json has been confirmed. The fileapps/example/index.jsexists and correctly initializes the application by importingAppand callingregisterRootComponent(App).package.json (1)
2-32: No major issues noted.These changes look consistent with a monorepo setup, and setting
"private": trueis appropriate if this package isn’t intended for public distribution.packages/react-native-device-activity/ios/ReactNativeDeviceActivityViewPersisted.swift (1)
1-97: Overall logic appears solid.The combination of Combine debouncing and local event dispatching is well-structured. Marking this as approved.
apps/example/screens/AllTheThings.tsx (4)
187-190: LGTM! Code simplification.The implementation now directly fetches the family activity selection using the selection ID rather than maintaining it in component state, which reduces state management complexity.
200-200: LGTM! Proper state management.Good addition of state for controlling the visibility of the activity picker component.
251-277: API usage improved with more intuitive methods.The changes replace the old blocking/unblocking implementation with more descriptive and function-specific methods:
blockAllApps,unblockAllApps,blockAppsWithSelectionId, andunblockSelectedApps. This improves code readability and maintainability.
332-363: LGTM! Enhanced UI with proper activity selection.The implementation of the activity picker with visibility control and appropriate callbacks is well done. The ability to reload the picker through the
onReloadcallback provides a good user experience when changes need to be refreshed.packages/react-native-device-activity/targets/ShieldAction/ShieldActionExtension.swift (4)
23-33: LGTM! Enhanced URL handling with flexible options.The addition of both
openUrlandopenUrlWithDispatchoptions provides good flexibility in URL handling, allowing for both immediate and asynchronous URL opening depending on the use case.
42-42: LGTM! Improved code maintainability.Using a constant (
CURRENT_UNBLOCKED_SELECTION) instead of a hardcoded string improves code maintainability and reduces the risk of typo errors.
59-59: LGTM! Consistent use of constants.Consistently using the same constant for retrieving and storing data ensures data integrity and improves code maintainability.
80-87: LGTM! Improved configuration retrieval.The use of
getActivitySelectionPrefixedConfigFromUserDefaultswith multiple parameters provides more flexibility in configuration retrieval, allowing for context-specific configurations based on application and web domain tokens.packages/react-native-device-activity/src/ReactNativeDeviceActivityModule.ts (3)
26-34: LGTM! Well-structured mock implementation.The
warnFnActivitySelectionWithMetadatafunction provides a consistent and structured mock response for activity selection metadata, which is important for testing and for providing fallback behavior on unsupported platforms.
47-51: LGTM! Consistent API mocking.These mock implementations for activity selection operations ensure the module provides consistent behavior across platforms, with appropriate warnings when used on unsupported platforms.
59-61: LGTM! Additional mock methods for app control.Adding mock implementations for
blockAllApps,unblockAllApps, andunblockSelectedAppsensures API consistency across platforms and provides appropriate warnings when used in unsupported environments.packages/react-native-device-activity/targets/ShieldConfiguration/ShieldConfigurationExtension.swift (5)
34-47: LGTM! Well-implemented image loading utility.The
loadImageFromAppGroupDirectoryfunction provides a clean way to load images from the app group directory with proper error handling, returning nil when the image cannot be loaded.
59-61: Image loading implementation now uses the new utility function.The code now uses the newly created
loadImageFromAppGroupDirectoryfunction for loading images from the app group directory, which provides consistent image loading behavior.
70-120: LGTM! Improved function signature and flexibility.Renaming
getShieldConfigurationtobuildShieldand modifying it to accept an external configuration parameter improves function flexibility and testability by decoupling the configuration retrieval from the shield building logic.
138-145: LGTM! Consistent approach to configuration retrieval.The updated function call uses the new
buildShieldfunction with configuration retrieved fromgetActivitySelectionPrefixedConfigFromUserDefaults, providing a standardized approach to configuration handling.
163-171: LGTM! Consistent pattern application.The same pattern is correctly applied to all shield configuration scenarios, ensuring consistent behavior across different contexts (applications, web domains, and categories).
Also applies to: 186-193, 211-219
apps/example/ios/reactnativedeviceactivityexample.xcodeproj/project.pbxproj (2)
874-874: Confirm the updatedREACT_NATIVE_PATH.Ensure that
"${PODS_ROOT}/../../../../node_modules/react-native"accurately reflects the updated monorepo structure and won’t break local developer environments or CI/CD pipelines.
936-936: Re-check path alignment across environments.Similar to the previous line, verify that this path modification does not disrupt or conflict with other build steps. Be sure that all references to
react-nativeacross the project remain consistent.packages/react-native-device-activity/ios/ReactNativeDeviceActivityModule.swift (5)
10-28: Shield state-check function looks good.The logic cleanly checks multiple shielded item types. Just ensure
storeis properly synchronized or locked if accessed concurrently. Otherwise, no immediate concerns.
572-587: Intersection function clarity.This function is straightforward, taking two serialized selections and returning metadata. Ensure you handle potential decoding errors in the underlying
deserializeFamilyActivitySelectionto avoid partial data or crashes.
588-603: Union operation.The union logic is correct. As with the intersection function, confirm that invalid or null inputs have a graceful handling path to avoid runtime exceptions.
620-635: Symmetric difference.Implementation follows the typical
symmetricDifferencepattern. No immediate concerns. Use test coverage for boundary cases.
713-715:blockAllAppshelper.Implements a catch-all blocking scenario. Ensure reusability by referencing it from higher-level functions that block multiple categories.
packages/react-native-device-activity/src/index.ts (8)
6-6: Imported persisted view.Importing the persisted variant is a logical addition. Verify that you’re consistently exposing it to consumers.
9-9: AddedActivitySelectionWithMetadatain import.Great for typed set operations. Ensure other modules or code referencing it remain in sync with any future changes.
18-18: Newly exported props interface.Exposing
DeviceActivitySelectionViewPersistedPropsis beneficial for typed usage. Keep it consistent with the actual persisted component’s required properties.
143-151: Intersection utility.Well-defined wrapper that delegates to the native module. Confirm that invalid or empty selections are handled gracefully to prevent unexpected null returns.
163-171: Difference utility.Straightforward difference operation. As with intersection and union, confirm you have coverage for duplicate tokens or large sets.
183-189: Retrieve selection metadata.Neat function to gather selection stats. Validate that the string is always properly serialized in calling code before usage.
507-508: Exporting both device activity selection views.Exposing these for broader usage in the app is helpful. Keep them properly documented for clarity on differences between the normal and persisted versions.
509-513: Export typed props for both views.Name alignment is good. The renaming clarifies usage in your codebase. No issues noted.
packages/react-native-device-activity/src/ReactNativeDeviceActivity.types.ts (11)
25-29: Type definition for ActivitySelectionMetadata looks good.Declaring these numeric properties is straightforward, and these counts intuitively capture the observed selection metadata. No further corrections needed here.
31-33: Ensure nullable type is appropriate for familyActivitySelection.By defining
familyActivitySelectionasstring | null, you handle situations where no family activity selection is active. Confirm that all call sites can handlenullvalues gracefully.
35-35: Renaming to ActivitySelectionWithMetadata aligns well with the new structure.Good job unifying the event type with the added metadata, ensuring consistency.
47-56: New DeviceActivitySelectionViewPersistedProps provides richer configuration.The added properties (like
includeEntireCategory) appear beneficial for more flexible UI handling, especially if entire categories need toggling.
211-213: Adding type "unblockSelection" with an explicit familyActivitySelectionId is clear.This design improves clarity by tying the unblock action directly to a specific selection identifier.
215-215: Introducing the "unblockAllApps" type is consistent with the other block/unblock methods.Nicely distinguishes between unblocking all apps vs. a subset.
246-260: Expanded doc comments for AuthorizationStatus are comprehensive.These comments improve clarity and help eliminate confusion around authorization states.
278-281: Exposing unblockSelectedApps with typed parameters encourages correct usage.The
familyActivitySelectionIdparameter ensures the call site supplies the appropriate identifier to target the selection.
284-303: New utility methods for activity selection metadata and set operations look solid.Methods like
intersection,union,difference, andsymmetricDifferenceprovide clear, modular manipulation of user selections. Verify that these are fully tested for edge cases.
311-311: blockAllApps method is clearly named and consistent with similar methods.This helps unify the approach to blocking logic within the module.
317-317: unblockAllApps method aligns nicely with the new blockAllApps approach.It extends the same clarity for comprehensive unblocking across the entire selection range.
packages/react-native-device-activity/ios/Shared.swift (14)
16-22: New user defaults keys improve clarity for selection-based configurations.Defining separate prefixes and flags helps avoid collisions and keeps the code more organized.
35-71: updateShield now supports activitySelectionId, enhancing fine-grained updates.Saving shield settings to both a default key and a selection-specific key fosters better reuse and scoping.
318-332: getFamilyActivitySelectionIds returns a non-optional array, reducing null checks.This change consolidates selection retrieval logic and ensures an always-initialized collection.
336-346: getFamilyActivitySelectionById neatly deserializes the stored selection.This allows calling code to retrieve specific family activity selections by identifier. Good usage of user defaults references.
349-364: setFamilyActivitySelectionById function organizes selection storage.Storing serialized selection strings per ID greatly simplifies future lookups and modifications.
367-413: getActivitySelectionPrefixedConfigFromUserDefaults is a robust way to look up per-selection configs.It dynamically finds a matching selection entry before falling back to the default key. This approach should handle multi-selection environments well.
415-446: getPossibleFamilyActivitySelectionId function adds valuable reverse lookup ability.This method helps in quickly finding a selection ID related to a specific token or category.
526-547: intersection function is straightforward and properly merges tokens.Clean set-based logic, ensuring only overlapping activity tokens remain.
550-572: symmetricDifference function correctly captures tokens exclusive to each selection.This helps highlight items that don’t appear in both sets simultaneously.
575-596: difference function reliably subtracts tokens from the first selection.The return type is straightforward and fosters easy composition with other set operations.
599-620: union function properly combines tokens from both selections.This enables aggregate blocking or unblocking across multiple sets in a concise manner.
623-690: blockSelectedApps effectively coordinates partial blocking states with user defaults.By preserving
CURRENT_BLOCKED_SELECTIONandCURRENT_UNBLOCKED_SELECTION, the function ensures consistent store updates for new or merged selections.
693-760: unblockSelectedApps aligns with blockSelectedApps, maintaining consistent store usage.Swapping from blocked to unblocked sets, especially when partially blocking or unblocking, appears well handled.
975-994: Refined logic in shouldExecuteAction for skipIfLargerEventRecordedSinceIntervalStarted is clear.The code gracefully checks interval start triggers to ensure actions are not overshadowed by higher-priority events.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
.github/workflows/pr.yml (1)
27-28: 🧹 Nitpick (assertive)Add newline at the end of the file.
The file is missing a newline character at the end, which is causing the YAMLlint error.
- run: bunx pkg-pr-new publish working-directory: packages/react-native-device-activity +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 28-28: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/pr.yml(1 hunks)apps/example/ios/reactnativedeviceactivityexample.xcodeproj/project.pbxproj(3 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/pr.yml
[error] 26-26: trailing spaces
(trailing-spaces)
[error] 28-28: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Swift Test (example project)
- GitHub Check: SwiftLint (example project)
🔇 Additional comments (2)
.github/workflows/pr.yml (1)
25-25: Using working directory ensures commands run in the correct context.Specifying the working directory for the
prepublishOnlycommand is good practice and ensures it runs in the correct context after project reorganization.apps/example/ios/reactnativedeviceactivityexample.xcodeproj/project.pbxproj (1)
105-108: Good move from absolute to relative paths.The change from absolute paths to relative paths for Swift files improves project portability. The paths now correctly reflect the reorganized project structure, making the project more robust when cloned on different machines.
There was a problem hiding this comment.
Actionable comments posted: 20
🔭 Outside diff range comments (1)
apps/example/ios/Tests/SkipTests.swift (1)
98-304: 🧹 Nitpick (assertive)Well-organized test suite for interval-based skipping
TheSkipIfAlreadyTriggeredBetweenTestsclass effectively demonstrates a time-window-based skip condition. Consider adding edge-case tests around boundary timestamps.
♻️ Duplicate comments (2)
.github/workflows/pr.yml (2)
21-21: 🧹 Nitpick (assertive)Remove trailing spaces.
There are trailing spaces at the end of line 21 that should be removed, as flagged by YAMLlint.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 21-21: trailing spaces
(trailing-spaces)
22-22: 🧹 Nitpick (assertive)Add newline at end of file.
Add a newline at the end of the file to fix the YAMLlint error.
- run: npx pkg-pr-new publish './packages/*' +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
.github/workflows/pr.yml(1 hunks)apps/example/Tests.xctestplan(1 hunks)apps/example/ios/Tests/ComparisonTests.swift(1 hunks)apps/example/ios/Tests/FamilySelectionIdTests.swift(1 hunks)apps/example/ios/Tests/SkipTests.swift(4 hunks)apps/example/ios/Tests/TextReplacementTests.swift(1 hunks)apps/example/ios/reactnativedeviceactivityexample.xcodeproj/project.pbxproj(4 hunks)apps/example/ios/reactnativedeviceactivityexample.xcodeproj/xcshareddata/xcschemes/Tests.xcscheme(1 hunks)packages/react-native-device-activity/ios/ReactNativeDeviceActivityView.swift(2 hunks)packages/react-native-device-activity/ios/ReactNativeDeviceActivityViewPersisted.swift(1 hunks)packages/react-native-device-activity/ios/Shared.swift(12 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/pr.yml
[error] 21-21: trailing spaces
(trailing-spaces)
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Swift Test (example project)
- GitHub Check: SwiftLint (example project)
🔇 Additional comments (36)
.github/workflows/pr.yml (2)
20-20: Working directory properly specified.Adding the working directory ensures the prepublishOnly script runs in the correct context for the react-native-device-activity package. This is important for proper package preparation during the publishing process.
22-22: Path argument correctly specifies target packages.The addition of the './packages/*' path argument to the pkg-pr-new publish command ensures that all packages in the packages directory are properly targeted, which aligns with the project's reorganized structure.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
apps/example/ios/reactnativedeviceactivityexample.xcodeproj/xcshareddata/xcschemes/Tests.xcscheme (1)
14-20: Test plan correctly integrated into the scheme.The addition of the TestPlans section with a reference to the new test plan file is appropriate. Marking it as the default test plan ensures it will be used automatically when running tests through this scheme.
apps/example/ios/Tests/FamilySelectionIdTests.swift (1)
11-16:Details
❓ Verification inconclusive
Ensure userDefaults is properly initialized.
The code uses
userDefaults?with an optional chaining operator, but the variable isn't defined in the visible code. Make sureuserDefaultsis properly declared and initialized, or consider usingUserDefaults.standarddirectly if appropriate.
🏁 Script executed:
#!/bin/bash # Search for userDefaults declaration in the codebase rg -t swift "var\s+userDefaults\s*[:=]"Length of output: 149
Verify the initialization and usage of the shared
userDefaultsin testsThe test file uses
userDefaults?without a local declaration. Our search shows a global declaration inpackages/react-native-device-activity/ios/Shared.swift:var userDefaults = UserDefaults(suiteName: appGroup)Please verify that the test target properly imports and accesses this global instance. If test isolation is a concern, or if the shared instance isn’t intended for test use, consider initializing a local instance (e.g., using
UserDefaults.standard) within the test.packages/react-native-device-activity/ios/ReactNativeDeviceActivityView.swift (1)
20-23: Ensure consistent lifecycle management for SwiftUI views.
ActivityPickerinitialization within aUIHostingControlleris appropriate. Verify that themodeldata flow remains consistent if you plan to dynamically supply or modify any additional SwiftUI states/properties (e.g., environment objects).apps/example/ios/Tests/TextReplacementTests.swift (6)
12-16: Avoid reliance on implicitly unwrapped optionals for userDefaults.Ensure
userDefaultsis properly initialized and consistently referenced. IfuserDefaultsis an optional, consider handling failures or applying dependency injection to reduce potential runtime issues.
24-30: Good coverage of basic string replacement.The test effectively validates a simple placeholder. Adding additional placeholders or edge cases (e.g., partial placeholder patterns) can strengthen coverage.
37-42: Consider verifying numeric edge cases.
"{asNumber:value}"works for "42", but a test for a non-numeric string, negative numbers, or floating-point strings (e.g.,"3.14") may further confirm robust handling.
44-56: Comprehensive testing of multiple placeholders.The test covers multiple placeholder types in one input. This is well-structured and demonstrates correct usage of the placeholder engine. Good job.
58-70: Maintain consistent type checks.Verifying that non-string values remain unchanged is solid. Continuing to confirm that only valid placeholders are replaced ensures high reliability. No issues here.
72-84: Excellent edge case coverage for nonexistent placeholders.The logic properly preserves placeholders with unrecognized or absent keys. This approach mitigates accidental data corruption.
apps/example/ios/Tests/ComparisonTests.swift (1)
12-20: Clarify numeric comparison behavior.
isHigherEvent(eventName: "5", higherThan: "10")returningfalseimplies string-to-integer conversion. Confirm robust error handling or fallback for non-numeric strings (e.g., "abc") if the function is used beyond these tests.packages/react-native-device-activity/ios/ReactNativeDeviceActivityViewPersisted.swift (2)
18-44: Debounce approach looks correct, verify potential side effects.The 0.1s debounce is reasonable, but double-check that no user interactions get lost due to short toggles within the debounce window.
46-48: Ensure proper sizing constraints for hosted SwiftUI.The
layoutSubviewsoverride sets the frame tobounds. If the view hierarchy changes size frequently, confirm that any auto layout constraints or parent resizing triggers re-layout as expected.apps/example/ios/Tests/SkipTests.swift (3)
3-46: Good test class naming for clarity
Renaming and structuring these tests with concise class names likeNeverTriggerBeforeTestshelps improve readability.
48-49: Ensure coverage of new test class
This newSkipIfTriggeredBeforeTestsclass is clearly named. Double-check that its tests cover both positive (skips) and negative (does not skip) scenarios.
306-307: SkipIfLargerTests class introduction
This class provides another dimension of skip logic based on comparing recorded events. Ensure large-event tests and borderline thresholds are fully exercised.apps/example/ios/reactnativedeviceactivityexample.xcodeproj/project.pbxproj (5)
105-108: Refactor relative paths for improved portability
These file references now use deeper relative paths. Verify that the new../../../structure resolves correctly in all development and CI environments.
109-109: Test plan file addition
IncludingTests.xctestplanin the project is a good step toward structured testing. Confirm that all relevant test targets are part of this plan.
226-226: Ensuring the new test plan group reference is valid
A9F920792D7988D400A179B2 /* Tests.xctestplan */was added to the group children. Double-check that the file is present and recognized by Xcode.
870-871: Inconsistent OTHER_LDFLAGS formatting
The single string formatOTHER_LDFLAGS = "$(inherited) ";still includes extra spacing. This comment repeats a past review concern.
929-930: Inconsistent OTHER_LDFLAGS formatting
Same as above:OTHER_LDFLAGScontains extra spaces and differs from other build configurations.packages/react-native-device-activity/ios/Shared.swift (14)
16-22: New constant definitions
These added constants (SHIELD_CONFIGURATION_FOR_SELECTION_PREFIX,SHIELD_ACTIONS_FOR_SELECTION_PREFIX, etc.) improve clarity around activity selection data. Usage looks straightforward and consistent with naming conventions.
35-71: ExtendedupdateShieldwithactivitySelectionId
The extra parameter and corresponding user defaults logic provide a convenient way to store shield configurations per activity selection. Ensure any callers have been updated in the codebase to supply the correctactivitySelectionId.
256-260: Refined placeholder handling
Reformatting the return statement improves readability. No issues identified.
318-332: Non-optional return forgetFamilyActivitySelectionIds
Switching from an optional array to a guaranteed empty array if no items exist is a safer, more consistent API design. Make sure callers are aware they’ll always receive a valid array.
336-337:getFamilyActivitySelectionByIduses new dictionary key
The function now referencesFAMILY_ACTIVITY_SELECTION_ID_KEY. Validate that all references align with the updated naming and that no old key usage remains.
359-376:setFamilyActivitySelectionByIdensures dictionary updates
The function merges into an existing dictionary or creates a new one. This approach seems correct; just ensure concurrency safety if multiple threads might write the same entries.
428-458:getPossibleFamilyActivitySelectionId
This function scans the stored selections to locate the first matching token. Consider clarifying the behavior if multiple selections match—currently it returns the first one.
490-511:unblockAllAppscleanup
Removing keys from user defaults and clearing the store is correct. Confirm that external logic does not rely on these keys once removed.
513-529:blockAllApps
The approach is consistent with other shield toggles. The user defaultsIS_BLOCKING_ALLis toggled, while category sets are fully blocked. This is straightforward.
531-553:intersection
Combining tokens withintersectionclarifies which apps or categories apply across both sets. Implementation is clean.
555-579:symmetricDifference
Your approach using set operations is correct. This function ensures projects can handle partial overlaps in tokens.
581-603:difference
Subtracts tokens from one selection based on another. Straightforward usage of the set API.
605-627:union
Rectifies tokens from two selections by uniting them. Implementation is consistent with the other set-based functions.
698-766:unblockSelectedApps
Expands the set of unblocked tokens, adjusting the categories accordingly. Similar logic to the block flow. Double-check partial unblocking edge cases for categories.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/pr.yml (1)
21-21: 🧹 Nitpick (assertive)Remove Trailing Spaces
YAMLlint flagged trailing spaces on this line. Please remove any trailing whitespace to adhere to project style guidelines.Suggested Diff:
- +(Ensure no whitespace remains on this otherwise blank line.)
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 21-21: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
.github/workflows/pr.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/pr.yml
[error] 21-21: trailing spaces
(trailing-spaces)
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Swift Test (example project)
- GitHub Check: SwiftLint (example project)
🔇 Additional comments (1)
.github/workflows/pr.yml (1)
19-20: Working Directory Configuration for Prepublish Command
The addition of theworking-directory: packages/react-native-device-activityunder thebun run prepublishOnlystep correctly targets the subdirectory where the command must execute. Please ensure this path remains accurate as the project structure evolves.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.gitignore(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Swift Test (example project)
- GitHub Check: SwiftLint (example project)
🔇 Additional comments (6)
.gitignore (6)
68-69: Verify the inclusion of Yarn version files.
The addition of!.yarn/versionsensures that Yarn version files are tracked rather than ignored. Please confirm that this exemption is intentional and aligns with your version management strategy.
82-89: Good addition: Native sensitive file patterns.
The new "Native" section correctly ignores file patterns such as*.orig.*,*.jks,*.p8,*.p12,*.key, and*.mobileprovision. This is a useful security measure to prevent accidental commits of sensitive data.
90-92: Approved: Metro health check ignore rule.
The ignore pattern for.metro-health-check*is appropriate to filter out temporary or health check files generated by Metro.
93-97: Clarify debug log patterns.
The "debug" section now includes wildcards fornpm-debug.*,yarn-debug.*, andyarn-error.*. Since similar log files (e.g.,npm-debug.log) are already excluded earlier, please confirm that this broader pattern is intentional to capture all variants.
[nitpick, verify]
102-104: Approved: Local environment files exclusion.
Ignoring.env*.localfiles is a sound security practice to prevent local configuration details from being committed.
105-107: Approved: TypeScript build info is properly excluded.
The addition of*.tsbuildinfoin the "typescript" section ensures that TypeScript build artifacts are not committed to the repository.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/react-native-device-activity/package.json (2)
33-38: 🧹 Nitpick (assertive)Dependency Version Specification
Within the dependencies block, note that"@kingstinct/expo-apple-targets"is set to"0". It is advisable to specify a version range (for example,"^0.0.0") to allow for compatible updates while avoiding unintended major changes.Proposed diff:
- "@kingstinct/expo-apple-targets": "0", + "@kingstinct/expo-apple-targets": "^0.0.0",
48-52: 🛠️ Refactor suggestionSpecify Peer Dependency Version Ranges
The peerDependencies are currently defined with"*"which allows any version. For better compatibility control and to prevent consumers from accidentally using unsupported versions, consider specifying minimum version ranges. For instance:- "peerDependencies": { - "expo": "*", - "react": "*", - "react-native": "*" - } + "peerDependencies": { + "expo": ">=51.0.0", + "react": ">=18.0.0", + "react-native": ">=0.70.0" + }This change helps ensure that users install versions known to work with your module.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
packages/react-native-device-activity/package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Swift Test (example project)
- GitHub Check: SwiftLint (example project)
🔇 Additional comments (3)
packages/react-native-device-activity/package.json (3)
7-17: Scripts Configuration Check
The scripts section is well organized and includes commands for build, cleanup, linting, testing, and opening platform-specific project folders. Ensure that the paths (e.g.,"xed example/ios"and"open -a \"Android Studio\" example/android") remain valid as the project structure evolves.
24-32: Repository and Related Metadata
The repository, bugs, author, license, and homepage fields are correctly set. No changes required here.
39-47: DevDependencies Pinning
The devDependencies are currently pinned to specific versions. This is generally acceptable for development tooling. Just ensure these versions are compatible with your overall build and test environments.
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
apps/example/screens/AllTheThings.tsx (1)
332-371: 🧹 Nitpick (assertive)Well-implemented user interface for activity selection.
The addition of a Pressable component with the ActivityPickerPersisted modal provides a clear and intuitive interface for users to select applications. The implementation includes proper visibility handling and event callbacks.
However, consider adding error handling for the selection change operation:
onSelectionChange={(event) => { console.log("selection changed", event.nativeEvent); + try { console.log( JSON.stringify( ReactNativeDeviceActivity.userDefaultsGet( "familyActivitySelectionIds", ), null, 2, ), ); + } catch (error) { + console.error("Failed to retrieve selection IDs:", error); + } }}
♻️ Duplicate comments (2)
.github/workflows/pr.yml (2)
24-24: 🧹 Nitpick (assertive)Remove trailing spaces.
There are trailing spaces at the end of line 24 that should be removed.
- +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 24-24: trailing spaces
(trailing-spaces)
26-26: 🧹 Nitpick (assertive)Add newline at end of file.
The file is missing a newline character at the end, which is a common linting requirement.
- run: bunx pkg-pr-new@latest publish './packages/*' + run: bunx pkg-pr-new@latest publish './packages/*' +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 26-26: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/pr.yml(1 hunks).vscode/settings.json(1 hunks)apps/example/ios/Tests/UnionTests.swift(1 hunks)apps/example/ios/reactnativedeviceactivityexample.xcodeproj/project.pbxproj(6 hunks)apps/example/screens/AllTheThings.tsx(6 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/pr.yml
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 26-26: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Swift Test (example project)
- GitHub Check: SwiftLint (example project)
🔇 Additional comments (25)
.vscode/settings.json (1)
5-5: Configuration Path Updated for SwiftLint
The SwiftLint executable path has been updated to"apps/example/ios/Pods/SwiftLint/swiftlint", which aligns with the new directory structure. This change ensures consistency with the related updates in the CI/CD and pre-commit configurations..github/workflows/pr.yml (1)
19-24: New build steps look good!The additional build steps properly prepare the package before publishing by ensuring the build and prepublishOnly scripts run in the correct context.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 24-24: trailing spaces
(trailing-spaces)
apps/example/ios/reactnativedeviceactivityexample.xcodeproj/project.pbxproj (6)
105-108: Swift file path updates look good.The file paths have been correctly updated to reflect the new project structure with packages and apps directories. Using
SOURCE_ROOTinstead of absolute paths improves portability.
870-871: Verify React Native path accessibility.The React Native path has been updated to accommodate the deeper directory structure, but a previous verification indicated this path might not resolve correctly in all environments.
Run the following script to verify if the React Native module can be found at the specified path:
#!/bin/bash # Check if the React Native path exists with the new directory structure echo "Checking if React Native module exists at the specified path..." EXPECTED_PATH="node_modules/react-native" # Check from multiple potential root locations for dir in . .. ../.. ../../.. ../../../..; do if [ -d "$dir/$EXPECTED_PATH" ]; then echo "✅ Found React Native at $dir/$EXPECTED_PATH" exit 0 fi done echo "❌ React Native not found at expected locations relative to current directory" # Specifically check the exact path format in the project file if [ -d "${PODS_ROOT}/../../../../node_modules/react-native" ]; then echo "✅ Path with PODS_ROOT variable would resolve correctly" else echo "❌ Path with PODS_ROOT variable would not resolve correctly" fi
929-930: Inconsistent OTHER_LDFLAGS formatting.The
OTHER_LDFLAGSvalue has been updated to a single string with trailing space ("$(inherited) "), which is inconsistent with other build configurations in the file that use array notation.This inconsistency was previously identified and should be addressed to maintain a consistent configuration style throughout the project file.
621-621: SwiftLint path has been updated correctly.The SwiftLint configuration path now correctly points to the root of the project with the new directory structure.
639-639: SwiftLint path updated consistently.The SwiftLint configuration path has been consistently updated in both build phases.
109-109: New test plan file added.The addition of the
Tests.xctestplanfile supports proper test configuration for the reorganized project.apps/example/screens/AllTheThings.tsx (5)
31-31: Consider using a more descriptive identifier or moving to constants.The
selectionIdis hardcoded as "some-id-3" which could make maintenance difficult. For better code organization and readability, consider moving this to a constants file or providing a more descriptive name that indicates its purpose.-const selectionId = "some-id-3"; +// Consider moving to a constants file +const ACTIVITY_SELECTION_ID = "example-activity-selection";
185-198: Improved state management for activity selection.The refactored implementation directly retrieves the family activity selection ID rather than maintaining it in component state, which simplifies the code and reduces potential state synchronization issues.
250-277: Clear API improvements for app blocking/unblocking functionality.The updated implementation uses more explicit and descriptive API methods:
blockAllApps()andunblockAllApps()for managing all applicationsblockAppsWithSelectionId()andunblockSelectedApps()for managing selected applicationsThese changes make the code's intent clearer and provide better separation of concerns.
15-15: Good modularization with component imports.Adding Pressable from react-native and importing the ActivityPickerPersisted component from a separate file shows good modularity and separation of concerns.
Also applies to: 25-26
200-200: Clear state management for UI visibility.The addition of a dedicated state variable for picker visibility follows React best practices for controlling component visibility.
apps/example/ios/Tests/UnionTests.swift (12)
26-32: LGTM! Good test for empty intersection.The test correctly verifies that the intersection of two empty selections results in no tokens.
34-40: LGTM! Good test for empty union.The test correctly verifies that the union of two empty selections results in no tokens.
42-51: LGTM! Good test for union with all categories.The test correctly verifies that the union of a selection with all categories and an empty selection contains the expected tokens.
53-65: LGTM! Good test for union of two different selections.The test correctly verifies that the union of two distinct selections (social and games categories) contains the expected tokens.
67-79: LGTM! Good test for difference between equivalent selections.The test correctly verifies that the difference between two selections containing all categories results in no tokens.
81-90: LGTM! Good test for empty difference.The test correctly verifies that the difference of two empty selections results in no tokens.
92-107: LGTM! Good test for difference of non-overlapping selections.The test correctly verifies that the difference between two non-overlapping selections (social and games) contains the expected tokens.
109-124: LGTM! Good test for difference between all categories and games.The test correctly verifies that the difference between a selection with all categories and one with only games contains the expected tokens.
126-135: LGTM! Good test for empty symmetric difference.The test correctly verifies that the symmetric difference of two empty selections results in no tokens.
137-152: LGTM! Good test for symmetric difference of non-overlapping selections.The test correctly verifies that the symmetric difference of two non-overlapping selections (social and games) contains the expected tokens.
154-169: LGTM! Good test for symmetric difference between games and all categories.The test correctly verifies that the symmetric difference between a selection with only games and one with all categories contains the expected tokens.
8-11:Details
❓ Verification inconclusive
Verify utility function visibility.
The tests depend on utility functions (
deserializeFamilyActivitySelection,intersection,union,difference,symmetricDifference), but these aren't defined in this file. Make sure these functions are properly exposed and accessible to these tests.
🏁 Script executed:
#!/bin/bash # Check if the required utility functions are defined and accessible # Look for the implementation of set operation functions rg -A 1 "func (intersection|union|difference|symmetricDifference)\(" --type swift # Look for deserializeFamilyActivitySelection implementation rg -A 1 "func deserializeFamilyActivitySelection" --type swiftLength of output: 1291
Action: Confirm that shared utility functions are accessible in the test target.
The utility functions (
deserializeFamilyActivitySelection,intersection,union,difference, andsymmetricDifference) are implemented inpackages/react-native-device-activity/ios/Shared.swift. Please ensure that these functions are exposed to your tests inapps/example/ios/Tests/UnionTests.swift—for example, by marking them with the appropriate access modifiers (or by using@testable import) so that the test target can reference them.
- The functions are present in
Shared.swiftas verified by the grep output.- Confirm that the module containing these functions is properly imported into your test target.
There was a problem hiding this comment.
Actionable comments posted: 27
♻️ Duplicate comments (2)
packages/react-native-device-activity/ios/Shared.swift (1)
416-426: 🛠️ Refactor suggestionPrevent accidental overwriting during ID renaming.
renameFamilyActivitySelectionId(previousId:newId:)copies data frompreviousIdtonewIdwithout checking ifnewIdalready exists. Existing data atnewIdwill be overwritten silently.Use a guard or condition to handle collisions gracefully:
if var familyActivitySelectionIds = userDefaults?.dictionary(forKey: FAMILY_ACTIVITY_SELECTION_ID_KEY) { + if familyActivitySelectionIds[newId] != nil { + logger.log(level: .error, "renameFamilyActivitySelectionId: key '\(newId)' already in use.") + return + } familyActivitySelectionIds[newId] = familyActivitySelectionIds[previousId] ... }packages/react-native-device-activity/targets/ShieldConfiguration/ShieldConfigurationExtension.swift (1)
34-47:⚠️ Potential issueGuard against nil appGroupDirectory
The new
loadImageFromAppGroupDirectoryfunction forcibly unwrapsappGroupDirectory!on line 37, which could cause a runtime crash if the app group directory is not available or properly configured.Consider safely unwrapping the optional value:
- let fileURL = appGroupDirectory!.appendingPathComponent(relativeFilePath) + guard let appGroupDirectory = appGroupDirectory else { + print("Error: No app group directory found.") + return nil + } + let fileURL = appGroupDirectory.appendingPathComponent(relativeFilePath)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
⛔ Files ignored due to path filters (2)
apps/example/ios/Podfile.lockis excluded by!**/*.lockbun.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
README.md(1 hunks)apps/example/app.json(2 hunks)apps/example/ios/Podfile(1 hunks)apps/example/ios/Podfile.properties.json(1 hunks)apps/example/ios/Tests/FamilySelectionIdTests.swift(1 hunks)apps/example/ios/reactnativedeviceactivityexample.xcodeproj/project.pbxproj(7 hunks)apps/example/package.json(1 hunks)packages/react-native-device-activity/ios/Shared.swift(1 hunks)packages/react-native-device-activity/package.json(1 hunks)packages/react-native-device-activity/src/ReactNativeDeviceActivity.types.ts(8 hunks)packages/react-native-device-activity/src/ReactNativeDeviceActivityModule.ios.ts(1 hunks)packages/react-native-device-activity/src/ReactNativeDeviceActivityModule.ts(1 hunks)packages/react-native-device-activity/src/index.ts(10 hunks)packages/react-native-device-activity/targets/ShieldAction/ShieldActionExtension.swift(1 hunks)packages/react-native-device-activity/targets/ShieldConfiguration/ShieldConfigurationExtension.swift(4 hunks)
🧰 Additional context used
🧬 Code Definitions (6)
packages/react-native-device-activity/src/ReactNativeDeviceActivityModule.ios.ts (1)
packages/react-native-device-activity/src/ReactNativeDeviceActivity.types.ts (1)
ReactNativeDeviceActivityNativeModule(343-448)
packages/react-native-device-activity/targets/ShieldConfiguration/ShieldConfigurationExtension.swift (1)
packages/react-native-device-activity/ios/Shared.swift (1)
getAppGroupDirectory(1246-1253)
packages/react-native-device-activity/targets/ShieldAction/ShieldActionExtension.swift (2)
packages/react-native-device-activity/src/index.ts (5)
disableBlockAllMode(442-445)unblockSelection(451-463)addSelectionToWhitelistAndUpdateBlock(135-147)resetBlocks(447-449)SHIELD_ACTIONS_KEY(536-536)packages/react-native-device-activity/ios/Shared.swift (2)
openUrl(81-90)sendNotification(100-174)
packages/react-native-device-activity/src/ReactNativeDeviceActivityModule.ts (1)
packages/react-native-device-activity/src/ReactNativeDeviceActivity.types.ts (1)
ReactNativeDeviceActivityNativeModule(343-448)
packages/react-native-device-activity/src/index.ts (1)
packages/react-native-device-activity/src/ReactNativeDeviceActivity.types.ts (8)
ActivitySelectionInput(305-317)ActivitySelectionInputWithBlocks(319-332)SetOperationOptions(338-341)ActivitySelectionMetadata(26-31)OnDeviceActivityDetectedListener(602-604)EventListenerMap(606-612)OnAuthorizationStatusChange(450-454)DeviceActivityMonitorEventPayload(13-15)
packages/react-native-device-activity/src/ReactNativeDeviceActivity.types.ts (1)
packages/react-native-device-activity/src/index.ts (2)
DeviceActivitySelectionViewProps(625-625)DeviceActivitySelectionViewPersistedProps(626-626)
🔇 Additional comments (60)
packages/react-native-device-activity/package.json (1)
2-3: Update Package Metadata for Rebranding
The current metadata uses"name": "react-native-device-activity"with version"0.4.19", which conflicts with previous rebranding suggestions (e.g.,"react-native-device-activity-mono"and version"1.0.0"). Please confirm if the rebranding should be applied here or if the current metadata is intentional.apps/example/package.json (4)
4-4: Confirm Main Entry Point Change
The main entry point has been updated to"index.js", which aligns with the new restructuring of the project. Verify that this change is intentional and that the new entry point correctly initializes the example app.
10-10: New Bundling Script Added
The inclusion of the"bundle": "INTERNALLY_TEST_EXAMPLE_PROJECT=true expo export"script indicates a new bundling process. Please ensure that this command is thoroughly tested and that it produces the expected bundle output in the new project structure.
15-24: Review Dependency Version Updates
The dependency versions for Expo, React, React Native, and related packages have been updated. The change of"react-native-device-activity"to"workspace:*"is consistent with workspace management. Please verify that these updates are aligned with the stability and compatibility requirements for the target platforms.
35-36: Validate Native Modules Directory Path
The"nativeModulesDir"path has been updated from".."to"../.."in the Expo configuration. Confirm that this updated directory structure correctly resolves the native modules for the example app in the reorganized project layout.packages/react-native-device-activity/src/ReactNativeDeviceActivity.types.ts (14)
26-31: Documentation for new metadata fields already addressed.
This addition mirrors a previous suggestion to add doc comments for newly introduced metadata properties and to clarify usage details forincludeEntireCategory.
33-37: Type composition approach looks good.
CombiningfamilyActivitySelectionwithActivitySelectionMetadatais a clean way to strengthen type safety without introducing extra boilerplate.
49-62: Persisted selection view props look consistent.
The doc comment forincludeEntireCategoryand optionalstyleprop usage are well-defined. No further issues noted.
160-168: Ensure thorough testing of new shield action types.
These action types align well with previous additions to the shield logic. Confirm that these branches are covered by tests.
176-179: Looks good.
No immediate concerns. The property appears self-explanatory, but you might consider adding a short doc comment in the future if usage scenarios become more complex.
219-249: Whitelist management logic previously addressed.
These whitelist-related action types were discussed in an earlier comment about fallback scenarios (e.g., empty whitelists) and return values for success/failure.
276-295: Authorization status docstring is clear.
The descriptive comments forAuthorizationStatusTypealign with recommended best practices. Looks good.
305-318: Exclusivity checks for ActivitySelectionInput were previously recommended.
Ensure runtime checks or other guards exist to handle scenarios where both IDs or tokens are provided.
319-333: Broad usage of ActivitySelectionInputWithBlocks continues to look fine.
Past recommendations covered verifying downstream usage and tests for these extended fields.
334-342: SetOperationOptions documentation previously noted.
Narrow scoping looks good; earlier suggestions to expand doc coverage or clarify token-stripping logic remain relevant at your discretion.
393-396: Renaming logic previously reviewed.
Prior commentary covered concurrency and caching concerns forrenameActivitySelection. No new issues.
405-429: Add/remove selection to/from block/whitelist coverage previously discussed.
Confirm fallback handling and success/failure signals, if relevant to app state updates.
433-434: Managed settings store concurrency was already flagged.
Earlier feedback referenced ensuring concurrency safety for heavy usage. No new concerns here.
436-448: Monitor methods appear sound.
Stopping, starting, and retrieving monitored activities is well-declared. No immediate improvements needed.apps/example/ios/Podfile.properties.json (1)
4-4: Deployment target version increment looks fine.
Changing from iOS 15.0 to 15.1 is minor but ensure your CI and build pipelines align to this requirement across the project.README.md (1)
91-91: Updated iOS deployment target in docs.
Raising the target to iOS 15.1 matches the new project requirements. This change is consistent with other updated configurations.apps/example/ios/Podfile (1)
31-31: Updated iOS deployment target to 15.1This change increases the minimum iOS version from 15.0 to 15.1. This is consistent with the updates in other project files (like app.json) and ensures a unified minimum iOS version requirement across the project.
packages/react-native-device-activity/src/ReactNativeDeviceActivityModule.ios.ts (1)
8-8: Type assertion added for better type safetyAdding the explicit type assertion ensures that the module is treated exactly as the
ReactNativeDeviceActivityNativeModuletype, even if TypeScript's inference from the generic parameter is slightly different. This provides better type checking and IDE support.apps/example/app.json (2)
35-35: Updated iOS deployment target to 15.1This change increases the minimum iOS version requirement from 15.0 to 15.1, consistent with the update in Podfile. This is a coordinated change across the project files.
47-47: Plugin file missing from updated pathOur checks indicate that the new plugin path referenced in
apps/example/app.jsondoes not point to an existing file, which may break the build process. Specifically:
- The file at
../../packages/react-native-device-activity/app.plugin.jswas not found.- References to this file also appear in
packages/react-native-device-activity/config-plugin/getAppGroupFromExpoConfig.js.Please verify the correct new location of
app.plugin.jsin your restructured project and update all references accordingly.#!/bin/bash # Verify that the plugin file exists at the specified path echo "Checking if the plugin file exists at the specified path..." ls -la ../../packages/react-native-device-activity/app.plugin.js || echo "File not found" # Check for any app.plugin.js files in the project to find potential correct locations echo "Searching for app.plugin.js files in the project..." find ../../ -name "app.plugin.js" -type fpackages/react-native-device-activity/targets/ShieldConfiguration/ShieldConfigurationExtension.swift (5)
70-120: Improved function design with explicit config parameterRenaming from
getShieldConfigurationtobuildShieldand adding a separateconfigparameter improves the function's design by:
- Making it more descriptive of what it actually does (builds a shield)
- Decoupling it from a specific data source, making it more reusable and testable
- Making the configuration source explicit rather than implied
This is a good architectural improvement.
129-147: Updated function calls to match new signatureThe function calls have been correctly updated to use the renamed
buildShieldfunction with the new configuration parameter. The configuration is now retrieved separately and passed explicitly to the function.
155-175: Updated function calls to match new signatureThe function calls have been correctly updated to use the renamed
buildShieldfunction with the new configuration parameter, maintaining the same behavior while using the improved function design.
181-199: Updated function calls to match new signatureThe function calls have been correctly updated to use the renamed
buildShieldfunction with the new configuration parameter, preserving the functionality while benefiting from the improved function design.
207-227: Updated function calls to match new signatureThe function calls have been correctly updated to use the renamed
buildShieldfunction with the new configuration parameter, properly adapting to the new signature while maintaining the same behavior.apps/example/ios/Tests/FamilySelectionIdTests.swift (4)
2-2: Follow Swift naming conventions for the test file.Swift filenames typically begin with an uppercase letter. This recommendation was also made previously.
28-34: Good coverage for a non-existent ID scenario.This test properly ensures that the function returns nil for unknown IDs.
36-54: Duplicate family activity selection IDs reduce the intended test coverage.As previously noted, using
"my-id"again before"my-id-2"overwrites the first selection rather than creating two distinct selections.
66-77: Removal and validation logic looks correct.This test thoroughly checks that the selection is removed and IDs list is updated.
packages/react-native-device-activity/targets/ShieldAction/ShieldActionExtension.swift (4)
12-23: Block-all mode naming is consistent.Handling the
"disableBlockAllMode"action by callingdisableBlockAllModematches expectations. Code readability is good.
30-61: Confirm concurrency safety when unblocking multiple selections.If multiple “unblockAllPossibleFamilyActivitySelections” actions run concurrently, user defaults or blocking logic could collide. Verify thread safety or add synchronization if needed.
103-114: Validate user-provided URLs for safety.As noted in a previous review, opening arbitrary URLs can lead to potential misuse or errors. Consider adding strong validation before calling
openUrl(...).
121-133: Consider concurrency on whitelisting.Whitelisting the current app is written to user defaults. If other simultaneous updates occur, potentially ensure safe reads/writes (e.g., atomic merges or locking).
apps/example/ios/reactnativedeviceactivityexample.xcodeproj/project.pbxproj (5)
105-109: Relative path references are well-aligned.Switching to relative paths (e.g.,
../../../packages/...) removes the risk of absolute path breakage.
228-228: Tests.xctestplan inclusion is beneficial.Adding a test plan simplifies maintaining and running multiple test configurations.
879-879: Active compilation conditions updated successfully.Including
DEBUGin the inherited Swift compilation conditions is standard, aiding debug builds without apparent issues.
754-789:Details
❓ Verification inconclusive
Confirm iOS deployment target bump.
Raising
IPHONEOS_DEPLOYMENT_TARGETto 15.1 may exclude older devices or require additional validations. Verify the impact on supported iOS versions.
🏁 Script executed:
#!/bin/bash # Checking references to IPHONEOS_DEPLOYMENT_TARGET in the project file grep -n "IPHONEOS_DEPLOYMENT_TARGET" apps/example/ios/reactnativedeviceactivityexample.xcodeproj/project.pbxprojLength of output: 635
Action Required: Confirm Consistency of iOS Deployment Targets
The Debug/Release configurations for the main target have been updated to use
IPHONEOS_DEPLOYMENT_TARGET = 15.1(lines 754 and 789). However, other sections in the project file still reference higher deployment targets (16.6, 17.6, and 18.0). Please verify that the mixed deployment targets are intentional and won’t lead to compatibility issues across different build configurations.
623-641:Details
❓ Verification inconclusive
Verify SwiftLint configuration path.
Confirm that
"${PROJECT_DIR}/../../../.swiftlint.yml"resolves correctly in all build environments and CI.
🏁 Script executed:
#!/bin/bash # Check if the .swiftlint.yml file is accessible from the computed path ls -l "${PROJECT_DIR}/../../../.swiftlint.yml"Length of output: 104
SwiftLint configuration path requires manual verification
The executed shell script returned:
ls: cannot access '/../../../.swiftlint.yml': No such file or directoryThis error suggests that the computed path (
${PROJECT_DIR}/../../../.swiftlint.yml) did not resolve correctly in the current shell environment—likely because thePROJECT_DIRvariable isn’t set as it would be in a typical Xcode build environment. Please manually confirm the following:
- Verify that
PROJECT_DIRis correctly defined in all build environments (local and CI) so that the relative path resolves as expected.- Ensure that the
.swiftlint.ymlfile exists at the computed location relative to the project directory.Adjust the configuration if necessary, keeping in mind the differences in environment setups between your local shell and Xcode/CI.
packages/react-native-device-activity/src/ReactNativeDeviceActivityModule.ts (2)
58-104: Mock implementation is consistent.The structure for
mockModulefields aligns well with the declared interface. This no-op fallback is a clean approach for non-iOS platforms. No issues found.
106-106: No concerns.packages/react-native-device-activity/src/index.ts (15)
6-12: Imports look good.These updated imports align well with the new set operation features and
DeviceActivitySelectionViewPersisted. No issues found.Also applies to: 21-21, 23-23, 26-27
109-125: Good specialized error handling.This function neatly logs known error messages for block mode conflicts and whitelist category issues, improving debug visibility. No concerns with the approach.
127-129: Optional: consider wrapping in try/catch.If
refreshManagedSettingsStore()might fail in the native layer, capturing the error helps maintain consistent error handling across the module.Do we have any known cases of
refreshManagedSettingsStorefailing? If so, let's adopt consistent error handling.
135-147: Well-handled bridging call.Error handling is in place to catch potential native exceptions. This approach fosters a more stable user experience.
149-161: Consistent approach to error handling.Similarly well-handled bridging call with
try/catch. Great job.
249-259: Final set operation function.Completes the standard set operations (intersection, union, difference, symmetricDifference). Looks correct.
269-275: Direct bridging function.Matches the native module signature. No issues found.
424-436: Well-structured bridging call.Using a try/catch block for consistent screen time error handling is a good approach.
451-463: Consistent bridging pattern.Error handling is included, matching other block-related calls.
473-473: Emitter instantiation check.This lazy load approach is typical for bridging. No issues found.
352-359: Listener for device activity detection.Guarding against a missing emitter is good practice. Implementation is consistent with the rest of the code.
578-592: Simplistic equality check.Using
symmetricDifferenceto confirm no difference is a concise and effective approach within this domain.
594-612: Implementation clarity.Comparing the intersection result to the subset’s metadata is straightforward and clear. No issues found.
622-622: Exporting new component.Ensures
DeviceActivitySelectionViewPersistedis publicly available. No issues found.
624-627: Consistency in type exports.Renaming or re-exporting to maintain uniform naming is a good approach.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
📒 Files selected for processing (1)
packages/react-native-device-activity/package.json(1 hunks)
🔇 Additional comments (1)
packages/react-native-device-activity/package.json (1)
1-3: Clarify Package Naming and Version
The package metadata currently uses"name": "react-native-device-activity"and"version": "0.4.20". However, previous review comments and rebranding requirements suggested changing the name to"react-native-device-activity-mono"and updating the version to"1.0.0". Please confirm whether to proceed with the rebranding, or if the current metadata is intentional.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/react-native-device-activity/ios/Shared.swift (1)
76-82: 🛠️ Refactor suggestionAvoid blocking the current thread with dispatch group wait.
If called on the main thread, this can freeze the UI. Consider using a non-blocking delay, such as
Thread.sleep(forTimeInterval:)or asynchronous approaches.-func sleep(ms: Int) { - let delay = DispatchTimeInterval.milliseconds(ms) - let group = DispatchGroup() - group.enter() - _ = group.wait(timeout: .now() + delay) +func sleep(ms: Int) { + Thread.sleep(forTimeInterval: Double(ms) / 1000.0) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
📒 Files selected for processing (2)
apps/example/ios/Tests/FamilySelectionIdTests.swift(1 hunks)packages/react-native-device-activity/ios/Shared.swift(1 hunks)
🧰 Additional context used
🪛 SwiftLint (0.57.0)
packages/react-native-device-activity/ios/Shared.swift
[Warning] 41-41: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 58-58: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 181-181: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 225-225: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 288-288: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 291-291: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 319-319: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 328-328: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 367-367: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 385-385: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 399-399: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 415-415: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 432-432: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 577-577: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 624-624: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 639-639: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 703-703: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 735-735: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 1034-1034: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 1156-1156: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 1181-1181: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 1214-1214: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 1232-1232: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
[Warning] 949-949: Collection literals should not have trailing commas
(trailing_comma)
🔇 Additional comments (12)
apps/example/ios/Tests/FamilySelectionIdTests.swift (12)
2-2: Rename the file to follow Swift conventions.This was previously flagged: Swift filenames typically start with an uppercase letter and match the test class name (e.g.,
FamilySelectionIdTests.swift).
18-26: Looks good!The test correctly adds one selection and verifies the resulting count is 1.
28-34: Looks good!Retrieving a non-existent selection returns
nilas expected.
36-54: Fix duplicate ID usage in this test.This was previously flagged: you’re adding the same ID ("my-id") twice before adding a different ID, which overwrites the original selection, so the test incorrectly expects 2 distinct selections.
56-64: Test logic is correct.Verifying the retrieved selection matches the saved one is handled properly.
66-77: Test removal verification looks good.Removing the selection and confirming the ID is gone and count is zero is well done.
79-100: No concerns here.Configuration prefix retrieval test is coherent and passes an empty dictionary as expected.
102-120: Test logic is consistent.Properly verifies that no config is found under the specified ID.
122-133: Good negative test scenario.Correctly ensures that when there's no activity selection ID, the config is nil.
135-155: Correct sorting test.It confirms the sorted order (“social”, “everything”) as intended.
157-182: Approach is solid.Filtering only matches for “games” plus verifying “everything” is also included is correct.
184-219: Approval for multiple-category sorting.Ensures “games”, “social-and-games”, then “everything” appears in the expected order.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
📒 Files selected for processing (1)
packages/react-native-device-activity/ios/Shared.swift(1 hunks)
🔇 Additional comments (7)
packages/react-native-device-activity/ios/Shared.swift (7)
16-23: Add short documentation for newly introduced constants.This is a previously raised suggestion. Each of these user defaults keys and prefixes (
FALLBACK_SHIELD_CONFIGURATION_KEY,SHIELD_CONFIGURATION_FOR_SELECTION_PREFIX, etc.) would benefit from brief doc comments or usage clarifications so future maintainers can easily identify their purpose.
36-72: Handle missing or incompatible shield configuration gracefully.This is a previously raised suggestion. If either
shieldConfiguration_\(shieldId)orshieldActions_\(shieldId)is missing from user defaults, consider logging or providing a fallback. Silent failures can cause confusion and make debugging difficult.
74-79: Avoid blocking the current thread with dispatch group wait.This is a previously raised suggestion. Using dispatch groups to force a synchronous delay blocks the current thread, which can freeze your app if called on the main thread. Consider a safer or asynchronous approach to prevent potential UI hangs.
178-181: Prevent potential runtime crash by safely unwrapping the URL.This is a previously raised suggestion. Force-unwrapping
URL(string: url)!can crash if the string is invalid. Use conditional binding or early returns to avoid runtime errors.- let url = URL(string: url)! + guard let validUrl = URL(string: url) else { + logger.log(level: .error, "Invalid URL: \(url)") + return URLSessionDataTask() // or a safer fallback + }
879-885: Add more context to these custom error structs.This is a previously raised suggestion.
WhitelistSelectionWithoutEntireCategoryErrorandTryingToBlockSelectionWhenBlockModeIsEnabledErrorare empty. Adding a message or localized description can help with debugging and error reporting.
916-939: Guard against concurrent calls toupdateBlock().This is a previously raised suggestion. Multiple rapid calls to
updateBlock(triggeredBy:)can lead to race conditions with user defaults andManagedSettingsStore. You might want to wrap these calls in a dispatch queue or lock to prevent state corruption.
656-682:Details
❓ Verification inconclusive
Verify test coverage for set operations like
symmetricDifference.Although the logic appears sound, set operations can have edge cases (e.g., sets with overlapping tokens). Ensure you have unit tests covering typical, empty, and maximal sets to confirm correctness.
🏁 Script executed:
#!/bin/bash # Searches for test references to symmetricDifference in the codebase # and displays those lines for review. Modify if needed. rg -A 8 'func test.*symmetricDifference'Length of output: 42
Please verify that unit tests adequately cover the set operations used in this file. Although the code logic for computing the symmetric differences on
applicationTokens,webDomainTokens, andcategoryTokensappears correct, the initial search did not return any tests confirming that edge cases—such as overlapping sets, empty sets, and maximal sets—are handled correctly.
- Confirm that tests exist (or add new tests) to validate the behavior of
symmetricDifferencefor typical, empty, and full set scenarios.- Revisit related test files and ensure that all these cases are covered to guarantee robustness.
…from build process
There was a problem hiding this comment.
Actionable comments posted: 19
🔭 Outside diff range comments (2)
scripts/nail-workspace-dependency-versions.ts (1)
1-56: 🧹 Nitpick (assertive)Preserve original formatting of package.json files.
The current implementation might change the formatting of the
package.jsonfiles. Consider using a solution that preserves the original formatting.You might want to consider using a library like
jsonfileorjson-stringify-pretty-compactthat can preserve the original formatting when writing back to the file. Alternatively, you could read the original file as a string, parse it, make your changes, and then use string manipulation to apply the changes while preserving the original format.README.md (1)
1-184: 🧹 Nitpick (assertive)Alignment with PR Objective: Referencing New Functions
The PR title mentions the addition of
activitySelectionMetadataand related set functions. However, these new functionalities are not explicitly documented or referenced in the README updates provided in this diff. It would be beneficial to add a dedicated section or update existing examples to include usage details for the newactivitySelectionMetadataand set functions so that users can readily understand and apply these features.
♻️ Duplicate comments (8)
packages/react-native-device-activity/src/index.ts (5)
424-435: Returning in a void-typed function.
This pattern was flagged in past comments. Removing thereturnfrom avoidfunction helps maintain clarity and meets typical lint rules.
438-448: Likewise, consider removingreturnstatements withinvoidfunctions.
Matches prior feedback.
450-462: Same note:unblockSelectiontyped as void but returns a native call.
This was already noted — removing thereturnis recommended for consistency.
105-107: 🛠️ Refactor suggestionAdd error handling for parity with similar bridging calls.
This function, likeclearWhitelistAndUpdateBlock, should consider handling errors for consistency.Example fix:
-export const clearWhitelist = () => { - return ReactNativeDeviceActivityModule?.clearWhitelist(); +export const clearWhitelist = (): void => { + try { + return ReactNativeDeviceActivityModule?.clearWhitelist(); + } catch (error) { + handleScreenTimeError(error); + } };
99-103: 🛠️ Refactor suggestionConsider adding error handling for consistency.
Currently, this function returns the native call directly but does not wrap it in atry/catch. Other similar functions (e.g.,addSelectionToWhitelistAndUpdateBlock) handle errors gracefully usinghandleScreenTimeError. Aligning these conventions can help maintain consistent error handling throughout.Below is a possible adjustment:
-export function clearWhitelistAndUpdateBlock(triggeredBy?: string) { - return ReactNativeDeviceActivityModule?.clearWhitelistAndUpdateBlock(triggeredBy); +export function clearWhitelistAndUpdateBlock(triggeredBy?: string) { + try { + return ReactNativeDeviceActivityModule?.clearWhitelistAndUpdateBlock(triggeredBy); + } catch (error) { + handleScreenTimeError(error); + } }packages/react-native-device-activity/package.json (3)
2-3: Verify Package Name and Version Rebranding Requirement
There is an inconsistency between prior rebranding suggestions and the current metadata. If rebranding is intended, please update the package name and version as follows:- "name": "react-native-device-activity", - "version": "0.4.20", + "name": "react-native-device-activity-mono", + "version": "1.0.0",If the current naming/versioning is intentional, a brief confirmation or rationale in the PR description would help clarify.
18-24: 🧹 Nitpick (assertive)Augment Keywords for Improved Discoverability
To better reflect the newly added functionalities, consider including keywords such as"activitySelectionMetadata"and"set functions". For example:"keywords": [ - "react-native", - "expo", - "react-native-device-activity", - "device-activity", - "screen-time" + "react-native", + "expo", + "react-native-device-activity", + "device-activity", + "screen-time", + "activitySelectionMetadata", + "set functions" ]This change would improve the package’s discoverability.
34-39: 🧹 Nitpick (assertive)Improve Dependency Version Specification
The dependencies are mostly well defined. However, for"@kingstinct/expo-apple-targets", using a version range can be beneficial to allow compatible bugfix and minor updates. Consider updating the version as suggested below:- "@kingstinct/expo-apple-targets": "0.1.19", + "@kingstinct/expo-apple-targets": "^0.1.19",This change helps prevent potential compatibility issues in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
📒 Files selected for processing (5)
README.md(4 hunks)apps/example/ios/reactnativedeviceactivityexample/AppDelegate.mm(1 hunks)packages/react-native-device-activity/package.json(1 hunks)packages/react-native-device-activity/src/index.ts(11 hunks)scripts/nail-workspace-dependency-versions.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
packages/react-native-device-activity/src/index.ts (1)
packages/react-native-device-activity/src/ReactNativeDeviceActivity.types.ts (10)
ActivitySelectionInput(305-317)ActivitySelectionInputWithBlocks(319-332)SetOperationOptions(338-341)ActivitySelectionMetadata(26-31)OnDeviceActivityDetectedListener(602-604)EventListenerMap(606-612)AuthorizationStatusType(292-295)AuthorizationStatus(276-283)OnAuthorizationStatusChange(450-454)DeviceActivityMonitorEventPayload(13-15)
🔇 Additional comments (26)
apps/example/ios/reactnativedeviceactivityexample/AppDelegate.mm (2)
11-14: Optimization for test execution - good approach!Adding this conditional check to bypass React Native initialization during unit tests is a good optimization. This will significantly improve test performance by skipping the unnecessary React Native setup when running unit tests, reducing test execution time.
3-3: Added whitespace - no impactThis is just a whitespace change with no functional impact.
README.md (1)
91-91: Deployment Target Update: Confirm iOS Version CompatibilityThe iOS deployment target has been updated from
"15.0"to"15.1"in the expo-build-properties configuration. This change appears to align with the updated requirements in your project files. Please double-check that all native modules and documentation reflect this update to avoid compatibility issues on devices running iOS 15.1 and above.packages/react-native-device-activity/src/index.ts (17)
1-1: No issues noted for this import.
6-6: Straightforward import ofDeviceActivitySelectionViewPersisted.
9-12: Additional imports look appropriate.
21-21: ImportingDeviceActivitySelectionViewPersistedPropsis consistent with the new usage below.
23-23: Successfully importingEventListenerMapfor typed event handling.
26-27: No concerns with importingOnDeviceActivityDetectedListenerandSetOperationOptions.
135-147: Ensure the function’s return type matches its usage.
IfaddSelectionToWhitelistAndUpdateBlockcan return a promise or perform asynchronous work, consider typing it accordingly (e.g.,Promise<void>). Otherwise, ensure the bridging call is truly synchronous.
149-161: Similarly verify return type correctness.
Same rationale asaddSelectionToWhitelistAndUpdateBlock: confirm whether to declare an async return or keep it synchronous.
472-472: Instantiation ofEventEmitterwith a generic is correct.
No concerns here.
497-498: Reasonable default values for polling attempts and interval.
500-501: Utility functionwait: no concerns.
502-526: Polling authorization status logic appears correct.
Checks status, waits, and exits on final attempt or if no longer not-determined. Looks fine.
548-549: Revised signature for event subscription is consistent.
563-563: Clean event listener bridging.
652-652: Export statement updated to includeDeviceActivitySelectionViewPersisted.
653-653: Extended type export is valid.
654-657: Re-exporting types with aliases is consistent with project patterns.packages/react-native-device-activity/package.json (6)
7-17: Scripts Section Review
The scripts provided (for building, cleaning, linting, type checking, testing, and opening project directories) are clear and appear to be appropriate. No changes are necessary here.
25-30: Repository and Issue Tracker Configuration
The repository and bugs URLs are correctly set up. They point to the appropriate GitHub locations.
31-33: Author, License, and Homepage Details
The author information, license, and homepage entries appear accurate and well-formatted.
40-48: DevDependencies Configuration Check
The devDependencies section appears appropriate for local development. Although"react": "*"and"react-native": "*"are quite permissive, if these are intentional to rely on the Expo-managed versions or for test environments, they are acceptable as-is.
49-53: Peer Dependencies Verification
The peerDependencies are correctly specified with minimum version requirements, ensuring compatibility with Expo, React, and React Native.
2-4:Details
❓ Verification inconclusive
Enhance Package Metadata with New Feature Details
The package’s basic metadata is set up correctly. However, since the PR objectives indicate additions such asactivitySelectionMetadataand set functions, it would be beneficial to update the description so that users immediately understand the new functionality.A suggested change for the description:
- "description": "Provides access to Apples DeviceActivity API", + "description": "Provides access to Apple's DeviceActivity API, featuring activitySelectionMetadata and set functions",Additionally, please confirm if any rebranding is planned. Previous review comments suggested updating the package name and version (e.g., to
"react-native-device-activity-mono"and"1.0.0"), but the current metadata uses"react-native-device-activity"and"0.4.20".
Enhance Package Metadata & Confirm Rebranding Intent
The current metadata in
packages/react-native-device-activity/package.jsoncorrectly declares the package name, version, and description. However, since the PR objectives include new functionalities—such asactivitySelectionMetadataand set functions—it would be beneficial to update the description so that users are immediately aware of the enhanced capabilities.Suggested change for the description:
- "description": "Provides access to Apples DeviceActivity API", + "description": "Provides access to Apple's DeviceActivity API, featuring activitySelectionMetadata and set functions",Additionally, please confirm if any rebranding is planned. Earlier review comments mentioned a potential update to the package name (e.g., to
"react-native-device-activity-mono") and version (e.g.,"1.0.0"). The current metadata still reflects"react-native-device-activity"and"0.4.20", so clarification on this point is needed.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/example/ios/reactnativedeviceactivityexample.xcodeproj/project.pbxproj (2)
105-108: Proper use of relative paths improves project portability.The paths to Swift files have been updated to use relative paths with SOURCE_ROOT instead of absolute paths. This change improves project portability across different development environments.
838-839: React Native path and linker flags formatting concerns.The React Native path has been updated to a deeper relative path structure, and OTHER_LDFLAGS includes an extra trailing space.
The verification scripts from previous reviews indicate that:
- The React Native path may not resolve correctly in all environments
- The OTHER_LDFLAGS formatting is inconsistent across build configurations
Consider standardizing the OTHER_LDFLAGS format across all configurations and verifying the React Native path resolves correctly in your CI environment.
Also applies to: 899-900
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
📒 Files selected for processing (2)
apps/example/ios/reactnativedeviceactivityexample.xcodeproj/project.pbxproj(8 hunks)packages/react-native-device-activity/ios/ReactNativeDeviceActivity.podspec(1 hunks)
🔇 Additional comments (5)
apps/example/ios/reactnativedeviceactivityexample.xcodeproj/project.pbxproj (5)
109-109: Test plan file added for structured testing.The addition of a test plan file indicates improved testing infrastructure, which is a good practice for maintaining code quality.
722-722: iOS deployment target update from 15.0 to 15.1.This change updates the minimum iOS version requirement. While it's a minor update, ensure this aligns with your app's compatibility requirements.
Please verify that this minimum iOS version change is intentional and consistent with your app's target audience and feature requirements.
Also applies to: 756-756
841-842: Swift compilation conditions added and version handling updated.Adding explicit Swift compilation conditions for debug builds is a good practice. The Swift version is now consistently set to 5.8 in both debug and release configurations.
Also applies to: 902-902
596-606: Updated resource copy script phase.The shell script phase for copying Pod resources has been updated with empty input/output paths. This is likely an automatic change from CocoaPods.
615-622: Updated framework embedding script phase.The shell script phase for embedding Pod frameworks has been updated with empty input/output paths. This is likely an automatic change from CocoaPods.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
apps/example/ios/Tests/FamilySelectionIdTests.swift (2)
2-2: Fix filename casing to follow Swift conventions.The filename "familySelectionIdTests.swift" starts with a lowercase letter, which doesn't follow Swift naming conventions. Swift filenames typically start with an uppercase letter. Consider renaming to "FamilySelectionIdTests.swift" to match the class name and follow standard Swift conventions.
36-54: Fix duplicate ID in testShouldHaveTwo.This test intends to verify that two distinct selections are properly counted, but you're setting the same ID ("my-id") twice before adding a different ID. This doesn't properly test the scenario since the first selection would be overwritten by the second.
func testShouldHaveTwo() { setFamilyActivitySelectionById( id: "my-id", activitySelection: FamilyActivitySelection() ) - setFamilyActivitySelectionById( - id: "my-id", - activitySelection: FamilyActivitySelection() - ) - setFamilyActivitySelectionById( id: "my-id-2", activitySelection: FamilyActivitySelection() ) let ids = getFamilyActivitySelectionIds() XCTAssertEqual(ids.count, 2) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
📒 Files selected for processing (2)
.github/workflows/test.yml(10 hunks)apps/example/ios/Tests/FamilySelectionIdTests.swift(1 hunks)
🔇 Additional comments (22)
apps/example/ios/Tests/FamilySelectionIdTests.swift (2)
192-192:Details
✅ Verification successful
Verify the union function implementation.
The
unionfunction is used here to combine two activity selections, but it's not defined in this file. Ensure it properly combines the selections with appropriate handling of duplicate entries.
🏁 Script executed:
#!/bin/bash # Search for the union function definition echo "Searching for union function definition..." rg "func\s+union\s*\(" --type swift -A 15Length of output: 1494
Union function verified
After verifying the implementation in
packages/react-native-device-activity/ios/Shared.swift, theunionfunction correctly combines twoFamilyActivitySelectioninstances by performing set unions onapplicationTokens,webDomainTokens, andcategoryTokens. The use of the built-in union operations ensures duplicate entries are properly handled. No further modifications are required. Please ensure that any future changes to the structure ofFamilyActivitySelectioncontinue to align with this union behavior.
82-82:Details
❓ Verification inconclusive
Verify that tokenIncludingSocial is defined.
The variable
tokenIncludingSocialis used here but doesn't appear to be defined in this file. Ensure it's properly defined elsewhere and available to this test class.
🏁 Script executed:
#!/bin/bash # Search for the definition of tokenIncludingSocial echo "Searching for tokenIncludingSocial definition..." rg "tokenIncludingSocial\s*=" --type swiftLength of output: 218
Ensuring Shared Token Accessibility:
The search confirms thattokenIncludingSocialis defined inapps/example/ios/Tests/UnionTests.swift(usinglet tokenIncludingSocial = ...). Please verify that this token is accessible inFamilySelectionIdTests.swift—either through an explicit import, a shared test helper file, or by being declared at a scope common to both test files—to prevent hidden dependencies..github/workflows/test.yml (20)
13-13: Set Timeout for Lint Job
The addition oftimeout-minutes: 10ensures that the lint job will be automatically stopped if it runs excessively long, which helps in keeping CI runs efficient.
28-28: Enforce Time Limit for Typecheck Job
Adding a 10‑minute timeout prevents the typecheck step from hanging indefinitely, aligning with overall CI best practices.
43-43: Timeout for Typecheck (Example Project) Job
A 10‑minute timeout has been set for the typecheck job of the example project. This appears consistent with other jobs.
54-54: Update Working Directory for Typecheck (Example Project)
Changing the working directory toapps/examplecorrectly reflects the new project structure. Please verify that all related file paths are updated accordingly.
59-59: Set Timeout for Expo Bundle Job
A 10‑minute timeout is now specified for the Expo Bundle job, which should help mitigate runaway processes.
70-70: Correct Working Directory for Expo Export
The update of the working directory toapps/examplefor the expo export step ensures consistency with the new directory layout.
74-74: Timeout for Expo Config Job
The addition of a 10‑minute timeout for the Expo Config job is consistent with other examples and helps maintain predictable CI durations.
86-86: Set Working Directory for Expo Config
Updating the working directory toapps/examplefor the expo config command aligns with the broader reorganization of the project structure.
91-91: Timeout for Expo Prebuild Job
Applying a 10‑minute timeout here prevents the Expo Prebuild job from overrunning, which is in line with the other project jobs.
102-102: Update Working Directory for Expo Prebuild
The change toworking-directory: apps/examplecorrectly reflects the new project layout for the expo prebuild command.
107-107: Set Timeout for SwiftLint Job
A 10‑minute timeout is now applied to the SwiftLint job (running on macOS), which is prudent given the potentially variable runtime of linting.
123-123: Confirm Installation Step in SwiftLint Job
The inclusion of thebun installcommand here helps ensure dependencies are up to date before linting. Verify that this remains consistent with dependency management practices across jobs.
125-125: Update Working Directory for Pod Install in SwiftLint
Changing the working directory toapps/example/iosfor the pod installation reflects the updated project structure and should help avoid path-related errors.
127-127: Explicit SwiftLint Command Path
Using the explicit pathapps/example/ios/Pods/SwiftLint/swiftlint lintensures that the locally installed SwiftLint is executed. This is a good move to avoid potential conflicts with any globally installed versions.
132-132: Extended Timeout for Swift Test Job
Setting the timeout to 25 minutes for the Swift Test job appears reasonable, likely accommodating longer-running tests in the iOS environment.
136-139: Node Setup in Swift Test Job – Verify Necessity
Although the project is transitioning to a Bun environment, the swift-test job still includes a step usingactions/setup-node@v4with Node version 22. Please confirm whether this Node.js setup is still required for the tests or if it can be removed in favor of a fully Bun‑based workflow.
169-170: Update Xcode Version for Swift Test Job
The Xcode version has been updated toXcode_16.2usingsudo xcode-select -s /Applications/Xcode_16.2.app/Contents/Developer. Make sure that all iOS build and test configurations are compatible with this version upgrade.
175-175: Update Working Directory for Pod Install in Swift Test
Updating theworking-directorytoapps/example/iosfor pod installation in the swift-test job is consistent with similar changes in the swift-lint job and the new project structure.
180-180: Confirm Working Directory for Xcodebuild Test
Theworking-directory: apps/example/iosspecified for the xcodebuild test command ensures the correct context for running tests. Verify that all related paths (e.g., the workspace and scheme names) are updated to match the new environment.
146-149:Details
❓ Verification inconclusive
Start Bundler in Swift Test Job – Process Management
The new "Start bundler" step runsbun start &with the working directory set toapps/example. While this change initiates the bundler process asynchronously, please ensure that:
- The background process is properly managed (e.g., that it does not remain hanging after tests complete).
- Subsequent steps that depend on bundler availability include proper synchronization (for example, adding a brief wait or health check).
A possible refinement could be:
- - name: Start bundler - run: bun start & - working-directory: apps/example + - name: Start bundler + run: | + nohup bun start & + sleep 5 # Allow bundler time to initialize + working-directory: apps/exampleConsider this adjustment if tests are sensitive to the bundler startup timing.
Bundler Process Management – Ensure Proper Synchronization and Clean-Up
The current workflow step starts the bundler asynchronously using
bun start &inapps/example, which might leave a lingering process if not properly handled. To avoid potential issues during or after the test execution, please ensure:
- The background bundler process is terminated once tests finish.
- Steps dependent on bundler startup include a delay or health check to confirm it’s running (e.g., using
sleepor a polling mechanism).A suggested improvement is as follows:
- - name: Start bundler - run: bun start & - working-directory: apps/example + - name: Start bundler + run: | + nohup bun start & + sleep 5 # Allow bundler time to initialize + working-directory: apps/exampleThis adjustment provides a basic synchronization mechanism, but please verify that it meets your test environment’s requirements and that the bundler process is cleaned up after tests complete.
2 participants
Summary by CodeRabbit
New Features
DeviceActivitySelectionViewPersistedcomponent for improved integration with device activity features.ReactNativeDeviceActivityModulewith new functions for managing activity selections and their metadata.Refactor
Tests
Chores
.gitignoreto cover a broader range of files and directories relevant to various development environments and tools.