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

fix(examples): allow project node_modules to be used #2200

Merged
merged 1 commit into from
Jun 21, 2024

Conversation

kkafar
Copy link
Member

@kkafar kkafar commented Jun 20, 2024

Description

The custom resolver implemented in our metro.config.js right now most likely does not conver all cases.
When working on my PR with modals for Android I've added jotai package and received errors from bundler that this package is not found,
because:

  1. it was not present in any nearby node_modules - the file that requries it resides now in apps/test-examples, thus application node_modules are not checked by default,
  2. only our custom resolver looks into apps node_modules & fails to resolve this package.

I haven't investigated why it does not work for jotai in particular, because more general solution is to just let the default module resolution algorithm look into
application node_modules as it should always do.

Changes

Added application node_modules to the list of additional node modules in metro config of each example application.

Test code and steps to reproduce

Install jotai w/o this change, require it in any example and try to run the application.
Then apply changes from this PR and see it works well.

Checklist

  • Ensured that CI passes

@kkafar kkafar requested a review from tboba June 21, 2024 08:31
@kkafar kkafar assigned alduzy and unassigned alduzy Jun 21, 2024
@kkafar kkafar requested a review from alduzy June 21, 2024 08:31
@alduzy
Copy link
Member

alduzy commented Jun 21, 2024

Looks good, if it fixes the crash then we should proceed.
I can still see react-native-restart import being underlined in apps/examples/App.tsx after I checkout to this branch, but I suppose it's a different problem to solve.

@kkafar
Copy link
Member Author

kkafar commented Jun 21, 2024

Could you document it? Cause I just checked and don't see an issue on my end. I'm proceeding here. As you said, if there is some other probelm we will handle it separately.

@kkafar kkafar merged commit 5301d3f into main Jun 21, 2024
7 of 8 checks passed
@kkafar kkafar deleted the @kkafar/restore-support-for-dependencies-in-examples branch June 21, 2024 18:22
alduzy pushed a commit that referenced this pull request Jun 28, 2024
## Description

The custom resolver implemented in our `metro.config.js` right now most
likely does not conver all cases.
When working on my PR with modals for Android I've added `jotai` package
and received errors from bundler that this package is not found,
because: 

1. it was not present in any nearby `node_modules` - the file that
requries it resides now in `apps/test-examples`, thus application
`node_modules` are not checked by default,
2. only our custom resolver looks into apps `node_modules` & fails to
resolve this package.

I haven't investigated why it does not work for `jotai` in particular,
because more general solution is to just let the default module
resolution algorithm look into
application `node_modules` as it should always do.


## Changes

Added application `node_modules` to the list of additional node modules
in metro config of each example application.

## Test code and steps to reproduce

Install `jotai` w/o this change, require it in any example and try to
run the application.
Then apply changes from this PR and see it works well.

## Checklist

- [ ] Ensured that CI passes
ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this pull request Oct 9, 2024
…n#2200)

## Description

The custom resolver implemented in our `metro.config.js` right now most
likely does not conver all cases.
When working on my PR with modals for Android I've added `jotai` package
and received errors from bundler that this package is not found,
because: 

1. it was not present in any nearby `node_modules` - the file that
requries it resides now in `apps/test-examples`, thus application
`node_modules` are not checked by default,
2. only our custom resolver looks into apps `node_modules` & fails to
resolve this package.

I haven't investigated why it does not work for `jotai` in particular,
because more general solution is to just let the default module
resolution algorithm look into
application `node_modules` as it should always do.


## Changes

Added application `node_modules` to the list of additional node modules
in metro config of each example application.

## Test code and steps to reproduce

Install `jotai` w/o this change, require it in any example and try to
run the application.
Then apply changes from this PR and see it works well.

## Checklist

- [ ] 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.

2 participants