Skip to content

chore(core): Got rid of setNative type errors #10768

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 21, 2025

Conversation

CatchABus
Copy link
Contributor

@CatchABus CatchABus commented Jul 20, 2025

PR Checklist

What is the new behavior?

This PR gets rid of type errors related to some computed setNative methods.
Since we use symbols to define methods with a single argument, TypeScript may complain about union string types for prototype's [prop: symbol].
Things can get a bad turn if some methods use any as an argument type or unions/enums end up in conflict with string type.

To solve this, the following actions have been performed:

  • Replaced argument any with the corresponding type
  • Replaced string enum with string type
  • Switched order of some computed method declarations to get rid of union type conflicts

A small example to understand the conflict:

	[testIDProperty.setNative](value: string) {
		this.setAccessibilityIdentifier(this.nativeViewProtected, value);
	}

        // This throws error as TS with cause enum to have conflict with expected string type
        [accessibilityStateProperty.setNative](value: AccessibilityState): void {
		this.accessibilityState = value as AccessibilityState;
		updateAccessibilityProperties(this);
	}

And this will work because TS doesn't know about expected string argument yet.

	[accessibilityStateProperty.setNative](value: AccessibilityState): void {
		this.accessibilityState = value as AccessibilityState;
		updateAccessibilityProperties(this);
	}
	
	[testIDProperty.setNative](value: string) {
		this.setAccessibilityIdentifier(this.nativeViewProtected, value);
	}

Copy link

nx-cloud bot commented Jul 20, 2025

View your CI Pipeline Execution ↗ for commit 02711c2

Command Status Duration Result
nx test apps-automated -c=android ✅ Succeeded 4m 32s View ↗
nx run-many --target=test --configuration=ci --... ✅ Succeeded 2s View ↗

☁️ Nx Cloud last updated this comment at 2025-07-20 22:30:08 UTC

Copy link

nx-cloud bot commented Jul 20, 2025

View your CI Pipeline Execution ↗ for commit d13dd20

Command Status Duration Result
nx run-many --target=test --configuration=ci --... ✅ Succeeded 1s View ↗

☁️ Nx Cloud last updated this comment at 2025-07-20 22:12:06 UTC

@CatchABus CatchABus marked this pull request as draft July 20, 2025 22:16
@CatchABus CatchABus marked this pull request as ready for review July 20, 2025 22:25
@NathanWalker NathanWalker merged commit 2377b6a into NativeScript:main Jul 21, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants