Updates to UnableToResolveError to account for different platforms. #267
Conversation
facebook-github-bot
added
the
CLA Signed
Codecov Report
@@ Coverage Diff @@
## master #267 +/- ##
==========================================
+ Coverage 85.68% 85.69% +<.01%
==========================================
Files 144 144
Lines 4556 4559 +3
Branches 712 712
==========================================
+ Hits 3904 3907 +3
Misses 582 582
Partials 70 70
Continue to review full report at Codecov.
|
gdborton
force-pushed
the
resolve-error-message
branch
from
0974c32 to
115d915
Compare
This error message is confusing when you get it on a mac, as /tmp/ isn't the tmp directory that is being used. The message should probably try to account for settings that are configured by users as well, the Metro Bundler cache at least is something that can be overwritten.
gdborton
force-pushed
the
resolve-error-message
branch
from
115d915 to
8a2eb05
Compare
| ); | ||
| const hasteCacheLocation = path.join( | ||
| os.tmpdir(), | ||
| 'haste-map-metro-4-*', |
There was a problem hiding this comment.
This changed from haste-map-react-native-packager-*, but ideally should come from the location where it's configured:
There was a problem hiding this comment.
We could get the actual cache path from jest-haste-map if it was exposed as a getter (from here).
For now, this is perfectly fine, but I would change the second part of hasteCacheLocation to haste-map-metro-* so we don't have to update the error message if we bump the JEST_HASTE_MAP_CACHE_BREAKER variable.
There was a problem hiding this comment.
Good call on removing the version, I'll update.
We don't have access to the HasteMap instance right? So we'd need to add it to module_map.js?
There was a problem hiding this comment.
Yeah, we could though change a bit the code here to have the HasteMap instance here once the getter for the cache folder gets eventually added to jest-haste-map.
There was a problem hiding this comment.
jest v24 is going to have a getter for the cache path 😃
The first alpha for v24 has just been published and we're going to update Metro to this alpha version soon
| ); | ||
| const hasteCacheLocation = path.join( | ||
| os.tmpdir(), | ||
| 'haste-map-metro-4-*', |
There was a problem hiding this comment.
We could get the actual cache path from jest-haste-map if it was exposed as a getter (from here).
For now, this is perfectly fine, but I would change the second part of hasteCacheLocation to haste-map-metro-* so we don't have to update the error message if we bump the JEST_HASTE_MAP_CACHE_BREAKER variable.
|
The function that @rafeca talked about is now part of Also, it seems like it makes sense to do the same for the Metro cache folder. Could we make it so we display the cache folder based on what Metro really uses instead of hardcoding it? |
|
Closing because of inactivity. |
Summary
This error message is confusing when you get it on a mac, as /tmp/ isn't the
tmp directory that is being used. The message should probably try to account
for settings that are configured by users as well, the Metro Bundler cache at
least is something that can be overwritten.
Test plan
Edited my node_modules/ folder for the metro-sample-app.