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

[MARKO-3] - <assign> and <var> tag migration #1158

Merged
merged 11 commits into from
Nov 26, 2018

Conversation

t-l9
Copy link

@t-l9 t-l9 commented Oct 26, 2018

Added <var> and <assign> migration transforms so users can still use the deprecated syntax, but allowing the reference to not be a breaking change in Marko V4.

Description

Both the <assign> and <var> tag have their attributes looped through to get transformed from a type of HtmlAttribute to a Scriptlet.

In the assign tag, one of two cases happen. Either the attribute name gets assigned a value, or the attribute name's value remains undefined. During this assignment, the new Scriptlet that is created is inserted as a sibling before the original elNode. After, the original elNode is detached.

The var tag has similar functionality to the assign tag. However, there are two differences. While looping through the attributes of the elNode, the attribute names are actually being defined as a variable and assigned rather than only being referenced. Again, during this assignment, the new Scriptlet that is created is inserted as a sibling before the original elNode. Before the detachment of the original elNode, it's children are looped over and inserted as a sibling before the original elNode to not lose the children that are referencing the declared variable.

Allows users to reference deprecated code without it breaking on compilation because the way the user wrote to code is transformed into how the code should be written.

Checklist:

  • I have read the CONTRIBUTING document and have signed (or will sign) the CLA.
  • I have updated/added documentation affected by my changes.
  • I have added tests to cover my changes.

@jsf-clabot
Copy link

jsf-clabot commented Oct 26, 2018

CLA assistant check
All committers have signed the CLA.

@DylanPiercey
Copy link
Contributor

@timlauter this looks good 😄! Would you mind updating the two failing snapshot tests to get the CI passing?

@codecov
Copy link

codecov bot commented Oct 26, 2018

Codecov Report

Merging #1158 into master will decrease coverage by 0.03%.
The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1158      +/-   ##
==========================================
- Coverage   90.52%   90.48%   -0.04%     
==========================================
  Files         311      311              
  Lines       11803    11808       +5     
==========================================
  Hits        10685    10685              
- Misses       1118     1123       +5
Impacted Files Coverage Δ
src/taglibs/core/assign-tag.js 85.71% <90%> (+2.38%) ⬆️
src/taglibs/core/var-tag.js 95.83% <94.44%> (-4.17%) ⬇️
src/compiler/ast/VariableDeclarator.js 74.07% <0%> (-11.12%) ⬇️
src/compiler/ast/UpdateExpression.js 87.5% <0%> (-2.5%) ⬇️

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 4578da9...623ab8e. Read the comment docs.

@coveralls
Copy link

coveralls commented Oct 26, 2018

Coverage Status

Coverage decreased (-0.04%) to 90.489% when pulling 623ab8e on assign-invoke-var-tag-migration into 4578da9 on master.

@DylanPiercey DylanPiercey merged commit f2d8c97 into master Nov 26, 2018
@DylanPiercey DylanPiercey deleted the assign-invoke-var-tag-migration branch November 30, 2018 20:00
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.

5 participants