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

[PLAT-6128] RN-CLI: Use a more exhaustive search for MainApplication.java #1365

Merged
merged 19 commits into from
Apr 28, 2021

Conversation

bengourley
Copy link
Contributor

Ejected Expo projects have a different directory structure to RN projects generated with the RN CLI. This update uses the glob package to determine the location of MainApplication.java no matter where it exists within the app directory.

Goal

Ensure the CLI works with the default directory structure of an ejected Expo app.

Design

The glob module was added to recursively search the app directory for MainApplication.java. This means the CLI can cope with various directory structures.

Changeset

  • Added glob dependency
  • Updated unit test mocking

Testing

Relying on updated unit tests and existing automated tests.
As well as catching regressions on the existing fixture apps, I'm currently adding a new ejected Expo fixture to the automated tests to determine that the updated logic does works as intended.

…ion.java

Ejected Expo projects have a different directory structure to RN projects
generated with the RN CLI. This update uses the glob package to determine
the location of MainApplication.java no matter where it exists within the
app directory.
@github-actions
Copy link

github-actions bot commented Apr 21, 2021

@bugsnag/browser bundle size diff

Minified Minfied + Gzipped
Before 40.86 kB 12.61 kB
After 40.86 kB 12.61 kB
± No change No change

code coverage diff

Coverage values did not change👌.

Total:

Lines Branches Functions Statements
81.47%(-0.01%) 70.42%(+0%) 84.01%(+0%) 80.39%(+0%)

Generated by 🚫 dangerJS against b15479b

@bengourley bengourley force-pushed the bengourley/rn-cli-main-application-search branch from a879c0d to c3a630d Compare April 22, 2021 09:24
@bengourley bengourley force-pushed the bengourley/rn-cli-main-application-search branch from ce5f737 to 25ffde7 Compare April 22, 2021 11:22
@bengourley bengourley force-pushed the bengourley/rn-cli-main-application-search branch from 5da5b49 to 01e09c4 Compare April 22, 2021 14:49
@bengourley bengourley force-pushed the bengourley/rn-cli-main-application-search branch from 259b089 to f827015 Compare April 26, 2021 16:25
Copy link
Contributor

@imjoehaines imjoehaines left a comment

Choose a reason for hiding this comment

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

Changes look good — approved assuming fixing the timeout won't change much of the main focus of this PR

test/react-native-cli/features/steps/steps.rb Show resolved Hide resolved
test/react-native-cli/features/steps/steps.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@twometresteve twometresteve left a comment

Choose a reason for hiding this comment

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

No comments from me.

@bengourley bengourley merged commit a3957b0 into next Apr 28, 2021
@bengourley bengourley deleted the bengourley/rn-cli-main-application-search branch April 28, 2021 09:19
This was referenced May 5, 2021
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