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

New analysis updates #378

Merged
merged 9 commits into from
May 15, 2019
Merged

New analysis updates #378

merged 9 commits into from
May 15, 2019

Conversation

guybedford
Copy link
Contributor

@guybedford guybedford commented May 9, 2019

@guybedford guybedford requested a review from styfle as a code owner May 9, 2019 12:01
@guybedford
Copy link
Contributor Author

In addition - package resolutions are attempted for resolution at runtime before throwing a not found error.

@codecov-io
Copy link

codecov-io commented May 9, 2019

Codecov Report

Merging #378 into master will decrease coverage by 1.11%.
The diff coverage is 73.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #378      +/-   ##
==========================================
- Coverage   74.41%   73.29%   -1.12%     
==========================================
  Files          12       14       +2     
  Lines         387      397      +10     
==========================================
+ Hits          288      291       +3     
- Misses         99      106       +7
Impacted Files Coverage Δ
src/@@notfound.js 0% <0%> (ø)
src/loaders/notfound-loader.js 100% <100%> (ø)
src/index.js 83.12% <92.85%> (-0.83%) ⬇️

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 b5033ed...698583c. Read the comment docs.

@styfle
Copy link
Member

styfle commented May 14, 2019

This change also allows us to remove the entire code path around the custom externals function we have, which is currently double-resolving all package resolutions (and incurring the statting and performance costs of that). I haven't removed it here because the externals warning is a feature we need to consider if we want to remove, but I would personally be quite happy removing those logs too.

If these warnings are decreasing performance, we should remove them.

Like you mentioned before: if the module require is tried at runtime and not found, then there will be an error instead. I think this error is pretty self-explanatory.

Error: Cannot find module 'unknown'

@guybedford guybedford changed the title Make all module not found errors runtime errors instead of build time errors New analysis updates May 15, 2019
@guybedford guybedford changed the title New analysis updates New analysis updates [do not merge] May 15, 2019
@guybedford guybedford changed the title New analysis updates [do not merge] New analysis updates May 15, 2019
@guybedford guybedford merged commit ad69084 into master May 15, 2019
@guybedford guybedford deleted the runtime-notfound branch May 15, 2019 22:50
guybedford added a commit that referenced this pull request May 15, 2019
@styfle
Copy link
Member

styfle commented May 16, 2019

Can all those issues be closed now?

@guybedford
Copy link
Contributor Author

Yes, all those issues can be closed!

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.

4 participants