halfnelson / nativescript-source-to-jsx-def

Walk nativescript source to generate JSX types for `svelte-type-checker-vscode`
Other
4 stars 1 forks source link

[Discussion] handling of iOS- and Android-specific attributes #14

Open shirakaba opened 4 years ago

shirakaba commented 4 years ago

Just occurred to me – we'll need to consider this aspect. Some of these are concerns to delegate to the renderer, and others are concerns for this project's typings. Either way, we may find a need to introduce a flag to omit the ios and android props from the typings altogether.

Platform-specific styles

I found in the styling docs that NativeScript Core XML supports various ways to declare platform-specific styles:

Platform-specific stylesheets (styles.ios.css, styles.android.css)
Platform-specific markup blocks (<ios> ... </ios>, <android> ... </android>)
Platform-specific attributes (<Label ios:style="..." android:style="...")
Platform-specific CSS rules (.ios .mystyle { ... }, .android .mystyle { ... }) *requires plugin

Platform-specific properties

The general case: prefixes

NativeScript Core JS handles platform-specific attributes using an ios or android prefix:

import { ActionBar } from '@nativescript/core';
const ab = new ActionBar();
ab.iosIconRenderingMode = "automatic";
ab.androidDynamicElevationOffset = 1;

So in RNS, I intercept the ios and android attributes (which is sensible in the first place as you probably don't want users to override the ios and android setters that proxy to nativeView), and iterate over each prop and apply the appropriate prefix. I'm not married to this approach, but it does the job.

The exception: objects

I found one exception, and I believe this may be the only instance of this pattern in the NativeScript Core.

import { ActionItem } from '@nativescript/core';

const ai = new ActionItem();
ai.ios = {
    position: "left",
    systemIcon: 0,
};
ai.android = {
    position: "actionBarIfRoom",
    systemIcon: "ic_menu_search"
};
shirakaba commented 4 years ago

Just had a quick look at how ActionItem props are currently being handled (commit de3b801644957481b90322cd910f7100df5024c6).

Svelte

// ui/action-bar/action-bar.d.ts
type ActionItemAttributes = ViewBaseAttributes & {
    actionbar?: ActionBar;
    actionview?: View;
    android?: AndroidActionItemSettings;
    icon?: string;
    ios?: IOSActionItemSettings;
    onIconChange?: (args: PropertyChangeData) => void;
    onTap?: (args: EventData) => void;
    onTextChange?: (args: PropertyChangeData) => void;
    onVisibilityChange?: (args: PropertyChangeData) => void;
    text?: string;
    visibility?: string;
};

React

// ui/action-bar/action-bar.d.ts
export type ActionItemAttributes = ViewBaseAttributes & {
    actionBar?: ActionBar;
    actionView?: View;
    android?: AndroidActionItemSettings;
    icon?: string;
    ios?: IOSActionItemSettings;
    onIconChange?: (args: PropertyChangeData) => void;
    onTap?: (args: EventData) => void;
    onTextChange?: (args: PropertyChangeData) => void;
    onVisibilityChange?: (args: PropertyChangeData) => void;
    text?: string;
    visibility?: string;
};

TypeScript gets a little confused here.

<actionItem text={"AI"} ios={{ position: "right" as const, systemIcon: 4, foo: "bar" }}></actionItem>

It tries to satisfy both the ios?: any and ios?: IOSActionItemSettings types at the same time. So it's strict on the members of IOSActionItemSettings (namely position and systemIcon), but at the same time the any type means that it's quite happy for you to enter in arbitrary members like foo.

image

I think it's not the end of the world – it gives auto-completion and some type-safety, so there are bigger fish to fry – but a more accurate type would omit any in this case. I don't believe there are any more cases like ActionItem out there (i.e. that make use of the ios attribute name for ios-specific attribtues), however, so it's unclear whether to handle it as an exception or not.