Skip to content

Conversation

@raejin
Copy link
Contributor

@raejin raejin commented Nov 8, 2018

Summary
When making a request to metro server with .bundle suffix, if this source file was named with .jsx or .ts, metro isn't able to find the resource. Instead, an error message is shown indicating there isn't such file associated with .js extension.

This PR addresses this issue by passing the entryFile name in the URL into the module resolution system, which will handle various extensions gracefully.

Test plan

  • Test in the application, making a request to a source file with .jsx extension.

…e it

assumed entryFile will end with `*.js`, which is not always true.
Instead, we pass it into the resolution logic to correctly determine the
actual path to the entryFile.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 8, 2018
@codecov-io
Copy link

Codecov Report

Merging #310 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #310      +/-   ##
==========================================
+ Coverage   86.31%   86.32%   +0.01%     
==========================================
  Files         163      163              
  Lines        4916     4913       -3     
  Branches      753      751       -2     
==========================================
- Hits         4243     4241       -2     
+ Misses        600      599       -1     
  Partials       73       73
Impacted Files Coverage Δ
packages/metro/src/lib/parseOptionsFromUrl.js 100% <100%> (ø) ⬆️
packages/metro/src/HmrServer.js 92.18% <100%> (+0.38%) ⬆️
packages/metro/src/Server.js 87.08% <100%> (+0.27%) ⬆️
...metro/src/ModuleGraph/output/indexed-ram-bundle.js 100% <0%> (ø) ⬆️
...rc/ModuleGraph/output/multiple-files-ram-bundle.js 100% <0%> (ø) ⬆️
packages/metro/src/ModuleGraph/output/util.js 93.47% <0%> (+0.62%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 568ec61...dc4d5b4. Read the comment docs.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@mjesun has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

thymikee added a commit to thymikee/metro that referenced this pull request Mar 3, 2019
…ation

* upstream/master: (42 commits)
  Bump [email protected]
  Fix entryFile always is assumed to end with `.js` extension (facebook#310)
  Adds support for `publicPath` to enable serving assets from different locations. (facebook#299)
  Make dynamic import interoperable for CJS and ESM
  Add custom serializer (facebook#309)
  React sync for revisions 4773fdf...3ff2c7c
  metro-buck: disable inline IDs in dev mode
  Simplify helper function
  Rewrite traverseDependency-integration-tests to make them module resolution unit tests
  RN: Revert React 16.6 Sync
  React sync for revisions 4773fdf...bf9fadf
  Bump react-deep-force-update
  Use getFileIterator() from jest-haste-map
  Deploy Flow v0.85 to xplat/js
  metro-buck: add explicit import() prefetch feature
  jest: remove jasmine usages
  jest: upgrade to 24.0.0-alpha.4
  Fix source map loading in the remote debugger
  Fix metro visualizer transformation
  Update babel to version 7
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants