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: meta content templates #429

Merged
merged 4 commits into from Aug 3, 2019
Merged

fix: meta content templates #429

merged 4 commits into from Aug 3, 2019

Conversation

pimlie
Copy link
Collaborator

@pimlie pimlie commented Jul 31, 2019

Both in v1 as in v2 the meta content templates caused wrong content if you would eg run the exact same tests multiple times. This was mainly caused by the fact that templates where removed once used so if both parent and child had templates defined on the second run the other template would be used.

So instead of explicitly removing templates we now keep track if a template has been applied for that specific meta property by setting template to true so a template is never applied twice (this was found in #426).

This fix could potentially break advanced configs like eg chained templates through multiple components though (but not sure if that was even possible, never tried it), but I think its better to support basic functionality as in 426 then advanced configs for now.

@pimlie pimlie requested a review from manniL July 31, 2019 20:24
@codecov
Copy link

codecov bot commented Jul 31, 2019

Codecov Report

Merging #429 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #429   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          34     34           
  Lines         534    547   +13     
  Branches      149    153    +4     
=====================================
+ Hits          534    547   +13
Impacted Files Coverage Δ
src/client/updaters/tag.js 100% <ø> (ø) ⬆️
src/shared/getComponentOption.js 100% <100%> (ø) ⬆️
src/shared/constants.js 100% <100%> (ø) ⬆️
src/shared/merge.js 100% <100%> (ø) ⬆️
src/shared/template.js 100% <100%> (ø) ⬆️
src/shared/getMetaInfo.js 100% <100%> (ø) ⬆️
src/server/generators/tag.js 100% <100%> (ø) ⬆️

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 66d98ee...036c0ea. Read the comment docs.

src/shared/getMetaInfo.js Outdated Show resolved Hide resolved
@pimlie pimlie merged commit 6907f9a into master Aug 3, 2019
@pimlie pimlie deleted the fix-meta-templates branch August 3, 2019 08:44
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.

None yet

2 participants