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

Converge with upstream (react-native-typescript-template, and react-native/template) #32

Conversation

mikehardy
Copy link
Collaborator

I know you're on vacation, please feel free to let this sit, it is not urgent.

As I worked on this template both by layering a template on it, and building multiple work projects on it, the differences between the upstream templates, and the amount of code here became worrisome

I think it will be a real maintenance burden, and the absolute modules pathing is a brand new concept for most people, making the template require that much more to learn in order to use it, plus requiring the deletion of a bunch of boilerplate to use it (the components etc).

Related #29 - and the comments there

As I was going to remove the absolute paths I realized all the work I was doing was in code copied from the default New App Screen in react-native, and I thought, why are we repeating this? Why aren't we referencing it?

I looked at react-native-typescript-template, and they just use the whole thing by reference. So much easier!

When I tried to do that on web, there was one problem with an import of react-native/Libraries/DevTools/openUrlInBrowser which I solved with a tiny patch-package.

I attempted to solve it via a shim + webpack alias - and I still think that's possible, to just override the import with a shim vs a patch - but I was unsuccessful and the patch was easy so I went that direction.

Once I referenced that all via import instead of code here, the app was so small, it made sense to just do the navigator in the App.tsx for demonstration purposes, with the react-native-typescript-template used directly, then wrapped

When I pasted their code in, the styles did not match, so I just copied in their eslint/prettier in order to have the same style as the community template, and yarn lint:all && yarn test:all all pass.

Much less to maintain, fewer decisions to defend with regard to design

What do you think?

the upstream code we can reuse uses these settings, to maintain
character by character compatibility as much as possible, while allowing
for a clean lint run here, reuse their lint settings

this will decrease template maintenance burden here by allowing easy
ingesetion of upstream changes from react-native-template-typescript
this allows the upstream react-native 'NewAppScreen' to work on web
@mikehardy mikehardy force-pushed the @mikehardy/upstream-convergence branch from 1d72bc8 to b92cafc Compare November 28, 2021 18:19
@mikehardy
Copy link
Collaborator Author

Thank goodness for CI! Fixed the silly typo I had in the patch-package integration commit

@LunatiqueCoder
Copy link
Owner

This is actually a very nice improvement! I’ll merge this by tomorrow. Thanks 🚀

allows deletion of majority of template contents, allows use
of upstream react-native-typescript-template App.tsx almost without
change, just wraps it in a toy navigator with toy vector icons demo
@mikehardy mikehardy force-pushed the @mikehardy/upstream-convergence branch from b92cafc to 4c6e3c5 Compare November 28, 2021 18:39
@mikehardy
Copy link
Collaborator Author

mikehardy commented Nov 28, 2021

Ah cool!
This will help me because what I realized as soon as I started building real projects on it is there was so much to throw away before I could get started!
I also realized because my auth template integrates react-native-paper (which has a theme) with react-navigation (which has a theme) I need to integrate those and make a new theme provider plus I needed to make a localization provider to set the auth template languages on firebase but that's not a change I can push upstream here as it would be an immediate large complication vs a pretty pure template here., so I think with this change I'm probably set on the Luna template as foundation for a while :-)

Cheers

@LunatiqueCoder LunatiqueCoder changed the base branch from master to develop November 28, 2021 19:15
@LunatiqueCoder LunatiqueCoder self-requested a review November 28, 2021 19:17
@LunatiqueCoder LunatiqueCoder added the enhancement New feature or request label Nov 28, 2021
@LunatiqueCoder LunatiqueCoder changed the base branch from develop to master November 28, 2021 19:32
@LunatiqueCoder LunatiqueCoder changed the base branch from master to develop November 28, 2021 19:33
@LunatiqueCoder
Copy link
Owner

Ok, I think I've messed up the develop branch a bit. 😂 I'll try to see what's wrong and I'll also try to publish a new version by tonight.

@LunatiqueCoder LunatiqueCoder merged commit 8dc17b5 into LunatiqueCoder:develop Nov 28, 2021
@mikehardy
Copy link
Collaborator Author

haha - uh-oh, that's why I just dev right into main branch ;-). I was never sure what a develop branch was for really (also known as the "go go go go go!" style of development ;-) )

I have a few more tiny changes, all formatting-related coming

@mikehardy mikehardy mentioned this pull request Nov 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants