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: support build cache #371

Merged
merged 7 commits into from
Oct 13, 2020
Merged

fix: support build cache #371

merged 7 commits into from
Oct 13, 2020

Conversation

pi0
Copy link
Member

@pi0 pi0 commented Oct 10, 2020

Fix issues with nuxt build cache for static target.

Related issues: #367, #352, #353

This PR mainly fixes #353

When using nuxt generate and cache hit happens, we skip nuxt build step and directly generate. However as build:* hooks won't be called anymore (even tough buildModules are called!)

Attempts to fix

  1. 491f440 ensures sw.js is properly emitted (temporary fix)
  2. Run pwa modules both if _build flag exists (nuxt dev, nuxt build and first optional stage of nuxt generate) or target is static. Downside is we call modules twice when cache miss occurs but seems only way
  3. Directly emit files and generate template as even tough buildModules are called for generate's second stage builder is not loaded so webpack hook to emit assets and addTemplate does not work

Known issues

icon

Nuxt does not checks static/ dir (static/icon.png) for build cache. This means if icon is updated, module wasn't being called. By always running module this should be fixed

manifest

Relying on webpack hook means we can't generate updated manifest (in case icon hash changes)

meta

If meta depending on process.env, we won't regenerate it as of addTemplate builder dependency

workbox

sw.js file needs to be restored. Instead of 491f440 workaround, we can directly generate template without depending on nuxt builder

@codecov
Copy link

codecov bot commented Oct 12, 2020

Codecov Report

Merging #371 into master will increase coverage by 0.12%.
The diff coverage is 95.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #371      +/-   ##
==========================================
+ Coverage   84.75%   84.87%   +0.12%     
==========================================
  Files          11       12       +1     
  Lines         400      390      -10     
  Branches      119      119              
==========================================
- Hits          339      331       -8     
+ Misses         55       54       -1     
+ Partials        6        5       -1     
Impacted Files Coverage Δ
lib/manifest/module.js 90.90% <85.71%> (-2.20%) ⬇️
lib/module.js 83.33% <85.71%> (-4.91%) ⬇️
lib/workbox/options.js 90.19% <87.50%> (ø)
lib/icon/module.js 92.40% <95.45%> (-0.78%) ⬇️
lib/meta/module.js 78.35% <100.00%> (-0.60%) ⬇️
lib/meta/module.runtime.js 100.00% <100.00%> (ø)
lib/utils/index.js 92.30% <100.00%> (+3.84%) ⬆️
lib/workbox/module.js 100.00% <100.00%> (+4.54%) ⬆️
lib/workbox/utils.js 94.73% <100.00%> (+1.40%) ⬆️

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 0aceb41...fba3d3d. Read the comment docs.

Copy link
Member Author

@pi0 pi0 left a comment

Choose a reason for hiding this comment

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

I tried to locally test with different situations, Still changes by this PR might introduce some regressions. In that case please make an issue :)

@pi0 pi0 merged commit 9a825c9 into master Oct 13, 2020
@pi0 pi0 deleted the fix/build-cache branch October 13, 2020 09:05
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.

static/icon.png hash is not invalidated with target: static
1 participant