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 module loading undefined error #12571

Merged
merged 2 commits into from
Jun 27, 2018
Merged

Conversation

chrisnojima
Copy link
Contributor

@keybase/react-hackers
So we've been bitten by this error once before, its really a problem with the metro bundler (facebook/metro#152) where if you import a file multiple times it can freak out. In this case we already were importing config-gen (https://github.com/keybase/client/blob/master/shared/actions/teams.js#L25) but then we added another import of config-gen and didn't clean up the second one (https://github.com/keybase/client/blob/master/shared/actions/teams.js#L17). This is kind of annoying and hard to debug or notice.

I think a best practice for us would be to

  1. Don't ever add .js to imports (also causes this problem)
  2. When importing the -gen/constants/types files always use import *
  3. When adding an import see if its already imported

Updated RN will fix this issue but we're not there yet.
Also we might to relook at a imports sorting eslint plugin again. Ideally it'd sort by file name maybe so its more obvious when this happens and would keep the order stable as we pull more things in (vs sorting based on the contents of the import line)

@chrisnojima chrisnojima merged commit 8e0438e into master Jun 27, 2018
@chrisnojima chrisnojima deleted the nojima/DESKTOP-fix-mobile-build branch June 27, 2018 13:54
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