Compile TypeScript to JavaScript before publishing to npm

[hassankhan]

Prevents issues with TypeScript declaration generation when the consuming project has strict mode enabled:

Found 15 errors in 8 files.

Errors  Files
     2  node_modules/uniffi-bindgen-react-native/typescript/src/async-callbacks.ts:25
     1  node_modules/uniffi-bindgen-react-native/typescript/src/async-rust-call.ts:15
     1  node_modules/uniffi-bindgen-react-native/typescript/src/callbacks.ts:36
     1  node_modules/uniffi-bindgen-react-native/typescript/src/enums.ts:7
     1  node_modules/uniffi-bindgen-react-native/typescript/src/errors.ts:41
     5  node_modules/uniffi-bindgen-react-native/typescript/src/ffi-converters.ts:66
     3  node_modules/uniffi-bindgen-react-native/typescript/src/objects.ts:16
     1  node_modules/uniffi-bindgen-react-native/typescript/src/rust-call.ts:14
✖ Failed to build definition files.

Fixes #196

[Johennes]

Thanks a bunch! Somebody recently reported an error that involved type generation which this fix will prevent in future. 👍

[Johennes]

Error: Cannot find module '/home/runner/work/uniffi-bindgen-react-native/turbo-module/node_modules/uniffi-bindgen-react-native/lib/index.js'. Please verify that the package.json has a valid "main" entry

I think we might have to trigger yarn build during the CI job?

[hassankhan]

C/C++: /home/runner/work/uniffi-bindgen-react-native/turbo-module/android/../cpp/generated/foobar.hpp:5:10: fatal error: 'UniffiCallInvoker.h' file not found
C/C++: #include "UniffiCallInvoker.h"
C/C++:          ^~~~~~~~~~~~~~~~~~~~~
C/C++: 1 error generated.

[Johennes]

Hm, this looks like the error that occurs when the pod isn't linked correctly: https://jhugman.github.io/uniffi-bindgen-react-native/guides/troubleshooting.html#compiling-for-ios-gives-an-error-unifficallinvokerh-file-not-found

I'm not sure how your PR would interact with that though. 🤔

I'm currently afk and cannot check deeper, sadly.

[hassankhan]

@Johennes Appreciate the pointers, I'm looking into it but haven't yet had any luck. Will post back if I get anywhere 👍

[hassankhan]

Interestingly, I can't even build locally:

Clicking "Template is declared here" takes me to https://github.com/jhugman/uniffi-bindgen-react-native/blob/main/cpp/includes/Bridging.h

[jhugman]

@hassankhan : thank you so much for digging into this!

@Johennes Appreciate the pointers, I'm looking into it but haven't yet had any luck. Will post back if I get anywhere 👍

Just dropping in here while I'm off. A few things that have been rattling round my head thinking about this PR:

• There are a couple of places where require.resolve is used to find the location of the uniffi-bindgen-react-native directory. If you're changing the main entry, then I would expect that these places will need to change.
• since uniffi-bindgen-react-native is the tool itself written in Rust, as well as a Typescript and C++ runtime, would you be okay if we rename your proposed lib directory to be underneath the typescript directory, say typescript/dist? I don't mind changing the name of typescript/ --> js/ for all typescript to live there. What do you think?
• is the intention to check the generated Javascript into git, or is this only for npm publish? does this need a change in the npm.yml publish flow?
• I don't know about you, but I find testing this to be quite challenging. @Johennes has done wonders with this, in the scripts/test-turbo-modules.sh and .github/workflows/compat.yml action. Have you any good ideas how to verify that Compile TypeScript to JavaScript before publishing to npm #198 stays fixed?
• Does "compiling typescript to Javascript" mean that we're now expecting handwritten Javascript to be calling the generated Javascript, or is it still Typescript calling generated Javascript? If it is the former, then we'll be relying on C++ runtime errors if methods are called with the wrong types. I'm not sure how to do better than that, but it sounds pretty icky. What do you think?

[hassankhan]

Hi @jhugman, hope you've had a great Christmas 🎄

There are a couple of places where require.resolve is used to find the location of the uniffi-bindgen-react-native directory. If you're changing the main entry, then I would expect that these places will need to change.

I've opened #200 to address this.

since uniffi-bindgen-react-native is the tool itself written in Rust, as well as a Typescript and C++ runtime, would you be okay if we rename your proposed lib directory to be underneath the typescript directory, say typescript/dist? I don't mind changing the name of typescript/ --> js/ for all typescript to live there. What do you think?

Absolutely, that makes sense and I'll update the PR accordingly.

is the intention to check the generated Javascript into git, or is this only for npm publish? does this need a change in the npm.yml publish flow?

I've tried to set it up so that it won't require any change to the publishing workflow; I'm using the prepare package script so it should run as part of the publish step in npm.yml.

I don't know about you, but I find testing this to be quite challenging. @Johennes has done wonders with this, in the scripts/test-turbo-modules.sh and .github/workflows/compat.yml action. Have you any good ideas how to verify that #198 stays fixed?

(a) In the short term, we can run npm pack to pack a tarball and then set up a dummy library to ensure yarn prepare works

(b) It would be nice to have an E2E test suite that runs through the Introduction example programmatically i.e. create an app, create a turbo module, add Uniffi, get the calculator example building.

Does "compiling typescript to Javascript" mean that we're now expecting handwritten Javascript to be calling the generated Javascript, or is it still Typescript calling generated Javascript? If it is the former, then we'll be relying on C++ runtime errors if methods are called with the wrong types. I'm not sure how to do better than that, but it sounds pretty icky. What do you think?

The latter. This change would only compile this library's TypeScript into JavaScript and TypeScript declaration files; the generated files in a RN module would still be TypeScript, and the consuming app is assumed to be TypeScript too (doesn't have to be I suppose).

Thanks for the feedback!

[jhugman]

Have you any good ideas how to verify that #198 stays fixed?

(a) In the short term, we can run npm pack to pack a tarball and then set up a dummy library to ensure yarn prepare works
(b) It would be nice to have an E2E test suite that runs through the Introduction example programmatically i.e. create an app, create a turbo module, add Uniffi, get the calculator example building.

Have a look in ./scripts/test-turbo-modules.sh. That runs through the tutorial making a calculator turbo-module library. That seems a short step away from your ideal in (b). Would this be something you'd be interested in adding?

[hassankhan]

Have a look in ./scripts/test-turbo-modules.sh.

I've had a quick look; it seems like in scripts/test-turbo-modules.sh, we need to do a find-and-replace for each generated library on integration/fixtures/turbo-module-testing/App.tsx, replacing the import to "../../src" with "$PROJECT_SLUG" before copying it to the generated library's example app.

Would this be something you'd be interested in adding?

I'll try my best and see how far I get 👍

[jhugman]

Hey @hassankhan did we lose you!? Oh no!

[hassankhan]

Apologies, I've been busy with another project. Still hoping to get back to this in the next few weeks 😅