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

hot reloading #1

Closed
medv opened this issue Feb 5, 2024 · 5 comments
Closed

hot reloading #1

medv opened this issue Feb 5, 2024 · 5 comments

Comments

@medv
Copy link

medv commented Feb 5, 2024

Thanks for the reproduction, this does indeed work if I add @babel/runtime and react dependencies to the pkgs packages.
Does hot reloading work for you? After refreshing on web, all changes are recompiled and reflected correctly - but not during HMR for some reason. Yet to test it on native

@medv
Copy link
Author

medv commented Feb 5, 2024

Following this guide https://docs.expo.dev/router/installation/ resolves the hot reloading issue, with one caveat:
The package.json "main" field will not resolve correctly. This can be set to "index.js" (which itself imports 'expo-router/entry', as per the "SDK48" code block in this section). Setting it to "node_modules/expo-router/entry" also resolves correctly, so I am wondering if this hints at a deeper resolution problem? I am at the limits of my pnpm/metro-fu here

@medv
Copy link
Author

medv commented Feb 5, 2024

Update: running expo cli from the app directory seems to be unable to resolve metro-config for one: ie pnpx expo start --web and pnpx expo export -p web will both fail on resolving it. Adding these in the app's package.json scripts ie expo start --web and expo export -p web respectively, resolves the issue.

Getting curious now, unsure if you will have any time to answer this @lokshunhung, understandable if not but maybe myself and some others might pick something up from all this! Cheers

@lokshunhung
Copy link
Owner

lokshunhung commented Feb 5, 2024

Hi @medv as you've probably already figured out, to run expo commands you need to be inside the apps/mobile directory:

$ cd apps/mobile
$ pnpm run start
or you can do at the root of the project using this command: (click to expand)
$ pnpm --filter @lokshunhung/mobile exec expo start
See more about pnpm's filter here The idea is that you need to execute expo at the app project's root instead of the repository root directory

On the issue you're having with web, unfortunately I'm afraid that I'm unable to help you with that.
This pnpm monorepo setup used to work with web at an earlier version of expo, but when I upgraded the expo sdk, webpack unfortunately broke. Expo also mentioned that they will not be supporting webpack anymore, so I decided that it will not be worth the effort to investigate into fixing webpack, and had therefore removed running the project on web in this commit. If you're interested in using webpack, you might want to checkout earlier commits before I updated the expo sdk, and upgrade + fix the webpack config from there.

I do should note that expo seems to be recommending using their metro bundler solution for web and using expo router moving forward. I think I won't be integrating that into this repository since I am in favour of using react-navigation directly and frankly not a big fan of file based routing. Feel free to fork this repository and make the required changes to use expo router. If that works for you let me know too, I am happy to link your setup in this repo's README if you wish to :)


(Edit): Forgot to mention, yes hot reload (react fast refresh) works on mobile

@medv
Copy link
Author

medv commented Feb 5, 2024

Thanks for the reply!
Not using webpack at all, just cloned your repository and followed the guide linked in my issue - that is only adding expo-router. Metro is the one bundling both web and native.
For pnpm scripts, I am running those in the expo app directory. Only when I write them directly into the app's package.json and executing them from there does it actually begin resolving metro-config. Essentially, there seems to be something weird going on (between "main": "expo-router/entry" being unable to resolve while "main": "node_modules/expo-router/entry" does resolve, and pnpx expo cli scripts being unable to be ran from command line, but able to be executed as a package.json script).

Hope that clears it up. My issue is not really to do with web, but with problematic resolution with strange effects that might apply to all kinds of future work built on this solution : (

ps. I should clarify that I have a functioning nx integrated-style monorepo using pnpm that works without these issues, I would just prefer a leaner solution which yours looks to be!

@medv
Copy link
Author

medv commented Feb 6, 2024

Apologies for spamming, just documenting as I problemsolve. Hope it helps at least one other person that doesn't have the full mastery of this yet like myself.

✔️ Your suggestion of using pnpm exec works, pnpx for me does not. There is a difference in resolution between the two commands it seems, with exec being able to step through the node_modules structure to find the relevant binary to call. This probably worked in a nx integrated monorepo because there was only the root node_modules, whereas here we are using pnpm workspaces properly. Simply stop using pnpx or pnpm dlx to call expo or eas cli from within the project dir, and instead use pnpm exec or just write the script using the appropriate cli in the project's package.json

"main": "expo-router/entry" remains a mystery. It should be resolved just fine, it is installed in the very directory/project that metro is being executed from. "node_modules/expo-router/entry" works so this must be metro misconfiguration. Found a relevant pr that may have solutions in the near future, at the very least it may be on the right track. Any tools that allow us to modify paths and do things relative to the root is more ammunition to debug/fix issues like these. This also might have the solution for:

❌ source maps of pkgs imported in the app load great on initial render, then after HMR console prints a warning saying that the sourcemap for the imported module is unable to be found. There are semi-adjacent issues in expo and metro to do with sourcemaps. It's unclear whether this monorepo causes an issue, or the issue is upstream. Probably wise to wait and see, then fix once upstream looks like it definitely works.

Closing this because there are fewer issues introduced by this pattern than it seemed at first, though it is not 100% yet in the clear. If anyone does any more experimentation, or doesn't experience these issues - please give a heads up :) Thank you lokshunhung!

@medv medv closed this as completed Feb 6, 2024
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

No branches or pull requests

2 participants