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(builder): await for renderer to load resources #5341

Merged
merged 1 commit into from Mar 23, 2019
Merged

fix(builder): await for renderer to load resources #5341

merged 1 commit into from Mar 23, 2019

Conversation

pi0
Copy link
Member

@pi0 pi0 commented Mar 22, 2019

This is how nuxt module tests are written:

// BeforeAll
nuxt = new Nuxt(config)
await nuxt.ready()
await new Builder(nuxt).build()
await nuxt.listen(3000)

// Tests
// Fetch http://localhost:300/...

With async-fs (#5186) we silently broke this because after awaiting build() we don't wait on renderer to load resources from fs and immediately requesting to the server causes tests failing.

We didn't await because it is a webpack hook: https://github.com/nuxt/nuxt.js/blob/dev/packages/webpack/src/builder.js#L130

With this PR, for production build a build:resources hook will be emitted and awaited on build(). This prevents breaking tests.

@pi0 pi0 requested a review from a team March 22, 2019 12:51
@codecov-io
Copy link

Codecov Report

Merging #5341 into dev will decrease coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             dev    #5341      +/-   ##
=========================================
- Coverage   95.7%   95.66%   -0.04%     
=========================================
  Files         74       74              
  Lines       2513     2514       +1     
  Branches     639      639              
=========================================
  Hits        2405     2405              
- Misses        91       92       +1     
  Partials      17       17
Impacted Files Coverage Δ
packages/vue-renderer/src/renderer.js 91.58% <ø> (-0.5%) ⬇️
packages/webpack/src/builder.js 95% <100%> (+0.05%) ⬆️

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 5d7757b...da3cbab. Read the comment docs.

1 similar comment
@codecov-io
Copy link

codecov-io commented Mar 22, 2019

Codecov Report

Merging #5341 into dev will decrease coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             dev    #5341      +/-   ##
=========================================
- Coverage   95.7%   95.66%   -0.04%     
=========================================
  Files         74       74              
  Lines       2513     2514       +1     
  Branches     639      639              
=========================================
  Hits        2405     2405              
- Misses        91       92       +1     
  Partials      17       17
Impacted Files Coverage Δ
packages/vue-renderer/src/renderer.js 91.58% <ø> (-0.5%) ⬇️
packages/webpack/src/builder.js 95% <100%> (+0.05%) ⬆️

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 5d7757b...da3cbab. Read the comment docs.

@pi0 pi0 merged commit caf5198 into dev Mar 23, 2019
@pi0 pi0 deleted the fix/tests branch March 23, 2019 07:02
@pi0 pi0 mentioned this pull request Mar 23, 2019
@danielroe danielroe added the 2.x label Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants