Skip to content
This repository has been archived by the owner on Dec 5, 2022. It is now read-only.

Tailor fails to parse template with comments inside #191

Merged
merged 2 commits into from
Oct 20, 2017

Conversation

DeTeam
Copy link
Contributor

@DeTeam DeTeam commented Oct 19, 2017

For some reason tailor fails to parse templates with HTML comments inside.

Judging by the error message and some snippets in the code, it seem like handling of comments is different from how other tags are processed.

     TypeError: Cannot destructure property `slot` of 'undefined' or 'null'.
      at nodes.forEach.node (lib/transform.js:56:51)
      at Array.forEach (<anonymous>)
      at Transform._groupSlots (lib/transform.js:54:15)
      at Transform.applyTransforms (lib/transform.js:32:24)
      at Promise.resolve.then.transform (lib/parse-template.js:19:19)
      at <anonymous>

@DeTeam DeTeam added the bug label Oct 19, 2017
@codecov
Copy link

codecov bot commented Oct 20, 2017

Codecov Report

Merging #191 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #191   +/-   ##
=======================================
  Coverage   98.29%   98.29%           
=======================================
  Files          14       14           
  Lines         586      586           
  Branches      104      104           
=======================================
  Hits          576      576           
  Misses         10       10

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 681aed5...05eec49. Read the comment docs.

@ruiaraujo
Copy link
Contributor

👍

1 similar comment
@DeTeam
Copy link
Contributor Author

DeTeam commented Oct 20, 2017

👍

@DeTeam
Copy link
Contributor Author

DeTeam commented Oct 20, 2017

/cc @vigneshshanmugam, check once you're back :)

@DeTeam DeTeam merged commit 832f078 into master Oct 20, 2017
@DeTeam DeTeam deleted the tailor-parsting-failure branch October 20, 2017 09:43
@DeTeam DeTeam mentioned this pull request Oct 23, 2017
if (!treeAdapter.isTextNode(node)) {
if (
!treeAdapter.isTextNode(node) &&
!treeAdapter.isCommentNode(node)
Copy link
Collaborator

Choose a reason for hiding this comment

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

should not be handled in this way..

This fix mean would completely skip comment nodes from child templates which is not the expected output.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, so why do we handle text nodes like?

Tomorrow I will take a look on passing it through.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, so why do we handle text nodes like?

yup, that was a bit puzzling, thus was thinking it's ok to have comments handled the same way

Copy link
Collaborator

Choose a reason for hiding this comment

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

Totally forgot why i did it that way. Will update once i remember.

But, with the fix #195, we can even include text nodes(space) as part of child templates.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants