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.

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

React Native mono-repo structure #463

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

bjtrounson
Copy link
Collaborator

I've split the react native project into 4 projects uniffi, core, maplibreui and example.

I think I finally got the typescript definitions behaving enough for local development by pointing it at the ./src/index.tsx file, but am unsure whether the uniffi project an be published in it's current state with it's typescript definitions not 100% building. I've created an issue in the uniffi-bindgen-react-native repo to see if they have an ideas jhugman/uniffi-bindgen-react-native#220

I would like some suggestions on names for these separate projects as I don't like the ones I've currently chosen.

  • uniffi: ferrostar-rn-uniffi
  • core: ferrostar-rn-core
  • maplibreui: ferrostar-rn-maplibreui

Maybe we should name them something around these lines, but these seem quite long. Let me know what you think @ianthetechie

  • uniffi: @stadiamaps/ferrostar-react-native
  • core: @stadiamaps/ferrostar-react-native-core
  • maplibreui @stadiamaps/ferrostar-react-native-maplibreui

Still need to make sure that this thing is build-able in the example project and clean up some of the build stuff between all the projects as I know some of it can be merged to together. I've already started this by making the tsconfig.json files all reference a root config file.

Copy link
Contributor

@ianthetechie ianthetechie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good to me! Just a few minor things to clean up; otherwise I'm good to merge this after you're happy with it!

@@ -0,0 +1 @@
# core
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't need to be fancy, but let's at least put a few lines here :)

"ferrostar-rn-core": ["./src/index"]
}
},
"exclude": ["lib"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this auto-generated? I don't see anything at that path in the PR.

"extends": "../tsconfig.base",
"compilerOptions": {
"paths": {
"ferrostar-rn-core": ["./src/index"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh nice; I guess that lets us work a bit more conveniently in a multi-package "workspace!"

Comment on lines +36 to +38
"resolutions": {
"@types/react": "^18.2.44"
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... I guess there's some dependency web that means we have to specify a resolution here manually?

@@ -0,0 +1,78 @@
{
"name": "ferrostar-rn-core",
"version": "0.27.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add support for all the React Native files to update-release-version.sh?

"ferrostar-rn-maplibreui": ["./src/index"]
}
},
"exclude": ["lib"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this stay at the "root" of the react native tree? Moving it under UniFFI seems oddly specific.

"ferrostar-rn-uniffi": ["./src/index"]
}
},
"exclude": ["lib"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

Copy link
Collaborator

@Archdoog Archdoog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool. Generally speaking like where this is going 👍

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.

3 participants