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

feat(webpack,cli): standalone build mode #4661

Merged
merged 3 commits into from Jan 3, 2019

Conversation

Projects
None yet
7 participants
@pi0
Copy link
Member

pi0 commented Dec 30, 2018

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

This PR implements @clarkdo idea (#4645 (comment)) for standalone build mode and resolves #4645.

The build.standalone option is turned off by default to prevent any behaviour/breaking changes.

Benchmarks

  • Tested on bare-bone hello-world
  • Cold start time is measured using the production build of nuxt. From reading config until nuxt.ready()
  • Built on windows (Yeah I'm using windows again :P)

Normal build:

  • Size of server.js: 22 Kib
  • Cost of dependencies (vue, vue-router, lodash (/omit), vue-no-ssr, debug): 7.4M
  • Cold start: 306.385ms
  • Memory usage: 29.9 MB (RSS: 106 MB)

Standalone build:

  • Size of server.js: 145 Kib
  • cold start: 306.550ms
  • Memory usage: 28.6 MB (RSS: 106 MB)

Results prove that there should be no big runtime diffs with the standalone mode. Still, we need to benchmark nuxt against real-world projects to decide on the default value.

Remarks

  • This is strange what dependency adds lodash/omit to the server bundle. Standalone build tree shakes only needed parts but should be still addressed
  • We need to announce users to use nuxt build --standalone when they need to use it with nuxt-start.
    • Alternative: For now we can add dependencies again to the nuxt-start distribution but it makes package bigger again

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly. (PR: #)
  • I have added tests to cover my changes (if not applicable, please state why)
  • All new and existing tests are passing.

@pi0 pi0 requested review from aldarund , clarkdo and nuxt/core-team Dec 30, 2018

@pi0 pi0 assigned clarkdo and pi0 Dec 30, 2018

@pi0 pi0 referenced this pull request Dec 30, 2018

Closed

fix: nuxt-start cli error #4645

1 of 7 tasks complete
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Dec 31, 2018

Codecov Report

Merging #4661 into dev will decrease coverage by 0.03%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #4661      +/-   ##
==========================================
- Coverage   90.82%   90.79%   -0.04%     
==========================================
  Files          67       67              
  Lines        2223     2226       +3     
  Branches      543      545       +2     
==========================================
+ Hits         2019     2021       +2     
- Misses        185      186       +1     
  Partials       19       19
Impacted Files Coverage Δ
packages/config/src/config/build.js 85.71% <ø> (ø) ⬆️
packages/webpack/src/config/server.js 100% <100%> (ø) ⬆️
packages/cli/src/commands/build.js 80% <50%> (-3.34%) ⬇️
packages/vue-renderer/src/renderer.js 95.1% <0%> (-0.55%) ⬇️
packages/webpack/src/config/base.js 96.47% <0%> (+1.17%) ⬆️

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 d455408...8afb160. Read the comment docs.

@clarkdo

This comment has been minimized.

Copy link
Member

clarkdo commented Dec 31, 2018

In standalone mode, webpack will bundle all deps, but if 3rd party lib is using dynamic require, it may throw module not found error.

For example: consola requires winston, but winston is devDependency, so webpack will throw error when bundle winston into server bundle

@pi0

This comment has been minimized.

Copy link
Member

pi0 commented Dec 31, 2018

@clarkdo Yes. That's a known problem of webpack with Node.js libraries like consola. We can whitelist some in standalone mode. But i don't recommend because it is dangerous in terms of a standalone build.

@aldarund

This comment has been minimized.

Copy link
Collaborator

aldarund commented Jan 2, 2019

What about when someone dont want ( or just updated nuxt from 2.3 to 2.4 without changing anythign) to use standalone mode, but rather just use nuxt start as solo production dependency. Then there still will be problem from #4645.

@pi0

This comment has been minimized.

Copy link
Member

pi0 commented Jan 2, 2019

@aldarund I just mentioned this in PR body as an alternative option:

Alternative: For now we can add dependencies again to the nuxt-start distribution but it makes package bigger again

But I assume it is a soft breaking change. We can make nuxt-start smarter to detect non-standalone builds and warn users in case of missing deps

@galvez
Copy link
Member

galvez left a comment

This is absolutely great.

@pi0 pi0 dismissed stale reviews from Atinux, galvez, and aldarund via 02f470a Jan 3, 2019

@pi0 pi0 merged commit bdb6791 into dev Jan 3, 2019

2 of 4 checks passed

[ci.azure] nuxt.js queued
Details
ci/circleci: setup Your tests are queued behind your running builds
Details
Semantic Pull Request ready to be squashed
Details
security/snyk - package.json (Atinux) No new issues
Details

@pi0 pi0 deleted the feat/standalone branch Jan 3, 2019

@pi0

This comment has been minimized.

Copy link
Member

pi0 commented Jan 3, 2019

Updates:

  • Dependencies for alternative approach added right into nuxt-start package (c664e3d, b5a0f78)

  • The mentioned extra import for lodash removed from server.js. (6178c47). Reducing hello world's server size to 124 KiB (Saving 21KiB)

@Atinux

This comment has been minimized.

Copy link
Member

Atinux commented Jan 8, 2019

Great work @pi0

@manniL

This comment has been minimized.

Copy link
Member

manniL commented Jan 11, 2019

We need docs for that, right? 🤔

@pi0

This comment has been minimized.

Copy link
Member

pi0 commented Jan 11, 2019

@manniL please add todo but it is experimental for now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment