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

refact: example apps file structure and ts support #2174

Merged
merged 10 commits into from
Jun 11, 2024

Conversation

alduzy
Copy link
Member

@alduzy alduzy commented Jun 7, 2024

Description

This PR intents to simplify example and test example apps file structure and fix typescript support.

Changes

  • removed package.json inside common dir and adjusted metro config to work with those dependencies
  • relocated app.js file inside test examples common dir to match examples dir structure
  • added submodules paths to the root tsconfig and extended it in the example dirs

Test code and steps to reproduce

  • Build and check wether the example and test example app works well on both paper and fabric architectures.
  • Have a look at the imports in any file inside apps/ directory and enjoy less red marks + full typescript support for linked packages!

Checklist

@alduzy alduzy changed the title feat: tsconfig paths added refact: example app file structure and ts support Jun 7, 2024
@alduzy alduzy changed the title refact: example app file structure and ts support refact: example apps file structure and ts support Jun 7, 2024
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

Hey, provided that it works (the apps do build and metro bundles) I think we are 🟢

Thanks!

tsconfig.json Outdated Show resolved Hide resolved
tsconfig.json Outdated Show resolved Hide resolved
@alduzy alduzy requested review from WoLewicki and kkafar June 7, 2024 13:52
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

Fine for me

Copy link
Member

@WoLewicki WoLewicki left a comment

Choose a reason for hiding this comment

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

LGTM, just test it with all caches cleared to be sure it works correctly 🎉

@kkafar
Copy link
Member

kkafar commented Jun 8, 2024

I've triggered CI again as e2e Android tests & iOS build on Fabric have failed.

@alduzy
Copy link
Member Author

alduzy commented Jun 10, 2024

The failing CI check Test Android e2e / test has been successfully tested by me and @kkafar on local. I'm going to re-run this check once again in the evening, but it seems to be some github CI fallacy.

@alduzy
Copy link
Member Author

alduzy commented Jun 11, 2024

It's worth noticing that it's not always the same test case that fails: It's either e2e/examplesTests/events.e2e.ts see here or e2e/examplesTests/bottomTabs.e2e.ts see here failing independently

Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

Yeah, I think facing the fact that the tests do pass w/o issues in local env, I think we should not be blocked by our CI setup / just CI runners and proceed here.

We need to handle these CI issues somehow separately.

Comment on lines -53 to -58
if (moduleName.startsWith('@react-navigation')) {
// For some reason, react-navigation packages don't want to resolve from
// the project's node_modules, so we need to use standard Metro resolver.
return context.resolveRequest(context, moduleName, platform);
}

Copy link
Member

Choose a reason for hiding this comment

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

Why this has been removed? Were there any errors regarding resolving this module? I'm just curious 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

After deleting the package.json file from apps/examples this code had to be deleted as the app couldn't find the react navigation packages

@alduzy alduzy requested a review from tboba June 11, 2024 12:48
ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this pull request Oct 9, 2024
…2174)

## Description

This PR intents to simplify example and test example apps file structure
and fix typescript support.

## Changes

- removed package.json inside common dir and adjusted metro config to
work with those dependencies
- relocated app.js file inside test examples common dir to match
examples dir structure
- added submodules paths to the root tsconfig and extended it in the
example dirs

<!--

## Screenshots / GIFs

Here you can add screenshots / GIFs documenting your change.

You can add before / after section if you're changing some behavior.

### Before

### After

-->

## Test code and steps to reproduce

- Build and check wether the example and test example app works well on
both paper and fabric architectures.
- Have a look at the imports in any file inside apps/ directory and
enjoy less red marks + full typescript support for linked packages!

## Checklist

- [ ] Included code example that can be used to test this change
- [ ] Updated TS types
- [ ] Updated documentation: <!-- For adding new props to native-stack
-->
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx
- [ ] Ensured that CI passes
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.

5 participants