Skip to content

deprecate other @obj #825

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 17, 2025
Merged

deprecate other @obj #825

merged 2 commits into from
Jan 17, 2025

Conversation

Freddy03h
Copy link
Member

@Freddy03h Freddy03h commented Dec 13, 2024 •
edited
Loading

I deprecate some missing @obj on the project but I can't decide what to do with some of them.

Example with transform

type transform
@obj external perspective: (~perspective: float) => transform = ""
@obj external rotate: (~rotate: angle) => transform = ""
@obj external rotateX: (~rotateX: angle) => transform = ""
@obj external rotateY: (~rotateY: angle) => transform = ""
@obj external rotateZ: (~rotateZ: angle) => transform = ""
@obj external scale: (~scale: float) => transform = ""
@obj external scaleX: (~scaleX: float) => transform = ""
@obj external scaleY: (~scaleY: float) => transform = ""
@obj external translateX: (~translateX: size) => transform = ""
@obj external translateY: (~translateY: size) => transform = ""
@obj external skewX: (~skewX: angle) => transform = ""
@obj external skewY: (~skewY: angle) => transform = ""

In the react-native doc it's:

array of objects: {matrix: number[]}, {perspective: number},{rotate: string}, {rotateX: string}, {rotateY: string}, {rotateZ: string}, {scale: number}, {scaleX: number}, {scaleY: number}, {translateX: number}, {translateY: number}, {skewX: string}, {skewY: string} or string

We cannot type it with @unboxed so I only see two strategies:

  1. Record with optional fields
type transform = {
  rotate?: angle,
  scale?: float,
  translateX?: float,
  …
}

Pro: JS output is clean
Con: Can be error prone

  1. Variant
type transform2 =
  | Rotate({rotate: angle})
  | Scale({scale: float})
  | TranslateX({translateX: float})

Pro: Enforce type
Con: JS output an object with a TAG field (even if we can cutomize it with @tag)

What's your opinion on it @cknitt ?

@Freddy03h
Copy link
Member Author

@cknitt ?

@cknitt
Copy link
Member

cknitt commented Dec 26, 2024

Sorry for the late reply. I think we should stay with an abstract type for correctness.
Either keep things the way they are or maybe something like

type transform

type perspectiveTransform = {perspective: float}
external perspective: perspectiveTransform => transform = "%identity"

type rotateTransform = {rotate: angle}
external rotate: rotateTransform => transform = "%identity"
...

?

@Freddy03h
Copy link
Member Author

Thank's for your reply!

I don't think there's a big difference between those two syntaxes. It's not enough to justify a rewriting imho.

let transformStyle = [perspective(~perspective=2.0), scale(~scale=1.5)]
let transformStyle = [perspective({perspective: 2.0}), scale({scale: 1.5})]

@Freddy03h Freddy03h requested a review from cknitt December 29, 2024 18:32
@Freddy03h
Copy link
Member Author

Hi @cknitt, can I have a review? thanks :)

@Freddy03h Freddy03h merged commit 4bf53fd into main Jan 17, 2025
2 checks passed
@Freddy03h Freddy03h deleted the deprecate_obj branch January 17, 2025 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cknitt cknitt cknitt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Freddy03h @cknitt