Skip to content
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

Simplify C++ TM struct generation #41645

Closed
wants to merge 1 commit into from
Closed

Simplify C++ TM struct generation #41645

wants to merge 1 commit into from

Conversation

christophpurrer
Copy link
Contributor

Summary:
Right now, when defining concrete structs and Bridging headers for Cxx TMs we need to define their member types twice:

using ConstantsStruct =
    NativeCxxModuleExampleCxxBaseConstantsStruct<bool, int32_t, std::string>;

template <>
struct Bridging<ConstantsStruct>
    : NativeCxxModuleExampleCxxBaseConstantsStructBridging<
          bool,
          int32_t,
          std::string> {};

Now we only need to define those once

using ConstantsStruct =
    NativeCxxModuleExampleCxxConstantsStruct<bool, int32_t, std::string>;

template <>
struct Bridging<ConstantsStruct>
    : NativeCxxModuleExampleCxxConstantsStructBridging<ConstantsStruct> {};

This change keeps the existing base types untouched - but they will be removed in the next RN version.

Changelog: [Internal]

Differential Revision: D51571453

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 25, 2023
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D51571453

christophpurrer added a commit to christophpurrer/react-native-website that referenced this pull request Nov 25, 2023
@christophpurrer
Simplify C++ TM struct generation
6a7ec21 
Update documentation to match:
facebook/react-native#41645
@analysis-bot
Copy link

analysis-bot commented Nov 25, 2023 •
edited
Loading

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 17,680,998 -5
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 21,064,032 +12
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: b50a709
Branch: main

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D51571453

1 similar comment
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D51571453

christophpurrer added a commit to christophpurrer/react-native-macos that referenced this pull request Nov 27, 2023
@christophpurrer @facebook-github-bot
Simplify C++ TM struct generation (facebook#41645)
e9e7685 
Summary:

Right now, when defining concrete structs and Bridging headers for Cxx TMs we need to define their member types twice:
```
using ConstantsStruct =
    NativeCxxModuleExampleCxxBaseConstantsStruct<bool, int32_t, std::string>;

template <>
struct Bridging<ConstantsStruct>
    : NativeCxxModuleExampleCxxBaseConstantsStructBridging<
          bool,
          int32_t,
          std::string> {};
```
Now we only need to define those once
```
using ConstantsStruct =
    NativeCxxModuleExampleCxxConstantsStruct<bool, int32_t, std::string>;

template <>
struct Bridging<ConstantsStruct>
    : NativeCxxModuleExampleCxxConstantsStructBridging<ConstantsStruct> {};
```

This change keeps the existing base types untouched - but they will be removed in the next RN version.

Changelog: [Internal]

Reviewed By: rshest

Differential Revision: D51571453
@christophpurrer @facebook-github-bot
Simplify C++ TM struct generation (facebook#41645)
8335d51 
Summary:

Right now, when defining concrete structs and Bridging headers for Cxx TMs we need to define their member types twice:
```
using ConstantsStruct =
    NativeCxxModuleExampleCxxBaseConstantsStruct<bool, int32_t, std::string>;

template <>
struct Bridging<ConstantsStruct>
    : NativeCxxModuleExampleCxxBaseConstantsStructBridging<
          bool,
          int32_t,
          std::string> {};
```
Now we only need to define those once
```
using ConstantsStruct =
    NativeCxxModuleExampleCxxConstantsStruct<bool, int32_t, std::string>;

template <>
struct Bridging<ConstantsStruct>
    : NativeCxxModuleExampleCxxConstantsStructBridging<ConstantsStruct> {};
```

This change keeps the existing base types untouched - but they will be removed in the next RN version.

Changelog: [Internal]

Reviewed By: rshest

Differential Revision: D51571453
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D51571453

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Nov 27, 2023
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in ef9c164.

@christophpurrer christophpurrer mentioned this pull request Nov 27, 2023
Merged
christophpurrer added a commit to facebook/react-native-website that referenced this pull request Nov 27, 2023
@christophpurrer
Simplify C++ TM struct generation (#3930)
7396470 
Update documentation to match:
facebook/react-native#41645
@christophpurrer christophpurrer mentioned this pull request Dec 3, 2023
Merged
christophpurrer added a commit to christophpurrer/react-native-website that referenced this pull request Dec 7, 2023
@christophpurrer
Simplify C++ TM struct generation
d98d243 
Update documentation to match:
facebook/react-native#41645
@christophpurrer christophpurrer mentioned this pull request Dec 7, 2023
Merged
christophpurrer added a commit to facebook/react-native-website that referenced this pull request Dec 7, 2023
@christophpurrer
Simplify C++ TM struct generation (#3947)
9f7093c 
Update documentation to match:
facebook/react-native#41645
sunnylqm added a commit to reactnativecn/react-native-website that referenced this pull request Dec 10, 2023
@sunnylqm
Squashed commit of the following:
1f11d3d 
commit 6a28b05
Author: Kartik Shankhavaram <shankhavaramk@gmail.com>
Date:   Sat Dec 9 22:50:21 2023 +0800

    Change 'use' to 'lose' to signify that functions are still accessible (facebook#3949)

commit 822b280
Author: Alex Hunt <hello@alexhunt.io>
Date:   Fri Dec 8 13:00:46 2023 +0000

    [docs] Fix name for 0.73 versioned docs (facebook#3948)

commit 4818763
Author: Nick Gerleman <ngerlem@meta.com>
Date:   Thu Dec 7 10:56:32 2023 -0800

    Lint and typecheck examples against RN 0.73 (facebook#3946)

    * Lint and typecheck examples against RN 0.73

    Updates the harness for linting snack examples to the newest version of RN.

    * Update Netlify Node version

commit cc46589
Author: Nick Gerleman <ngerlem@meta.com>
Date:   Thu Dec 7 10:56:04 2023 -0800

    Add `alignContent: 'space-evenly'` (facebook#3888)

    * Add `alignContent: 'space-evenly'`

    Coming with 0.74

    facebook/react-native#41020

    * Update layout-props.md

commit 9f7093c
Author: Christoph Purrer <christophpurrer@gmail.com>
Date:   Thu Dec 7 14:06:00 2023 +0100

    Simplify C++ TM struct generation (facebook#3947)

    Update documentation to match:
    facebook/react-native#41645

commit 59278e7
Author: Nick Gerleman <ngerlem@meta.com>
Date:   Thu Dec 7 01:28:12 2023 -0800

    Add `--frozen-lockfile` to `yarn install` (facebook#3945)

    To catch cases where dependency changes require a lockfile change that wasn't comited alongside.

commit 06ba8ea
Author: Nick Gerleman <ngerlem@meta.com>
Date:   Thu Dec 7 00:59:10 2023 -0800

    Revert "fixed peer dependency issues (facebook#3942)" (facebook#3944)

    This reverts commit 1bcb60f.

commit 1bcb60f
Author: Shantanu Gupta <99300527+Shantanugupta43@users.noreply.github.com>
Date:   Thu Dec 7 08:52:32 2023 +0000

    fixed peer dependency issues (facebook#3942)

    * fixed peer dependency issues

    * removed lingering file

commit 79ff42a
Author: Zeya Peng <zeyap@users.noreply.github.com>
Date:   Wed Dec 6 14:20:06 2023 -0500

    Update native-components-android.md (facebook#3941)

commit f0c4b80
Author: Aike van den Brink <17004429+aikewoody@users.noreply.github.com>
Date:   Wed Dec 6 19:46:03 2023 +0100

    Update new-architecture-app-intro.md (facebook#3938)

    Fixed small spelling mistake

commit f89ab82
Author: Alex Hunt <hello@alexhunt.io>
Date:   Wed Dec 6 17:56:40 2023 +0000

    [docs] Cut version 0.73.0 (facebook#3940)

commit 83e816b
Author: Alex Hunt <hello@alexhunt.io>
Date:   Wed Dec 6 17:15:14 2023 +0000

    [blog] Add 0.73 announcement post (facebook#3931)

commit d8c78ad
Author: Thibault Malbranche <thibault.malbranche@epitech.eu>
Date:   Wed Dec 6 17:33:48 2023 +0100

    fix(Image): update link to correct file (facebook#3939)

commit 821172e
Author: Sarvar Rose <47109217+sarvarrose@users.noreply.github.com>
Date:   Mon Dec 4 16:20:11 2023 +0530

    correct type of Pressable `style` prop (facebook#3936)

commit b1379a7
Author: adriancuadrado <adriancuadradochavarria97@gmail.com>
Date:   Mon Dec 4 00:57:19 2023 +0100

    Update scrollview.md (facebook#3869)

    `pagingEnabled` **is** supported on Android. I tested it and it just seems to work.

commit 11258b1
Author: Alan <49978973+3Alan@users.noreply.github.com>
Date:   Mon Dec 4 07:55:12 2023 +0800

    fix: remove useless points (facebook#3934)

commit 8400279
Author: Christoph Purrer <christophpurrer@gmail.com>
Date:   Sun Dec 3 09:57:25 2023 +0100

    Revert "Simplify C++ TM struct generation (facebook#3930)" (facebook#3935)

    This reverts commit 7396470.

commit 7396470
Author: Christoph Purrer <christophpurrer@gmail.com>
Date:   Mon Nov 27 13:00:23 2023 +0100

    Simplify C++ TM struct generation (facebook#3930)

    Update documentation to match:
    facebook/react-native#41645

commit 5616873
Author: Bailey Lissington <54869395+llamington@users.noreply.github.com>
Date:   Mon Nov 27 22:33:50 2023 +1300

    Fixed grammatical error (facebook#3929)
Othinn pushed a commit to Othinn/react-native that referenced this pull request Jan 9, 2024
@christophpurrer @Othinn
Simplify C++ TM struct generation (facebook#41645)
c7d6a18 
Summary:
Pull Request resolved: facebook#41645

Right now, when defining concrete structs and Bridging headers for Cxx TMs we need to define their member types twice:
```
using ConstantsStruct =
    NativeCxxModuleExampleCxxBaseConstantsStruct<bool, int32_t, std::string>;

template <>
struct Bridging<ConstantsStruct>
    : NativeCxxModuleExampleCxxBaseConstantsStructBridging<
          bool,
          int32_t,
          std::string> {};
```
Now we only need to define those once
```
using ConstantsStruct =
    NativeCxxModuleExampleCxxConstantsStruct<bool, int32_t, std::string>;

template <>
struct Bridging<ConstantsStruct>
    : NativeCxxModuleExampleCxxConstantsStructBridging<ConstantsStruct> {};
```

This change keeps the existing base types untouched - but they will be removed in the next RN version.

Changelog: [Internal]

Reviewed By: rshest

Differential Revision: D51571453

fbshipit-source-id: 2783bd48bf786ffa80d322d06456b5d6f2d7ba8a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@christophpurrer @facebook-github-bot @analysis-bot