useless code in function jqLiteBuildFragment?

[deplay]

filepath:angular.js/src/jqLite.js

code:

function jqLiteBuildFragment(html, context) {
  var tmp, tag, wrap,
      fragment = context.createDocumentFragment(),
      nodes = [], i;

  if (jqLiteIsTextNode(html)) {
    // Convert non-html into a text node
    nodes.push(context.createTextNode(html));
  } else {
    // Convert html into DOM nodes


// This line is suspicious
  tmp =  tmp || fragment.appendChild(context.createElement("div"))



    tag = (TAG_NAME_REGEXP.exec(html) || ["", ""])[1].toLowerCase();
    wrap = wrapMap[tag] || wrapMap._default;
    tmp.innerHTML = wrap[1] + html.replace(XHTML_TAG_REGEXP, "<$1></$2>") + wrap[2];

    // Descend through wrappers to the right content
    i = wrap[0];
    while (i--) {
      tmp = tmp.lastChild;
    }

    nodes = concat(nodes, tmp.childNodes);

    tmp = fragment.firstChild;
    tmp.textContent = "";
  }

  // Remove wrapper from fragment
  fragment.textContent = "";
  fragment.innerHTML = ""; // Clear inner HTML
  forEach(nodes, function(node) {
    fragment.appendChild(node);
  });

  return fragment;
}

when the function jqLiteBuildFragment is excuted,
if the js vm run the italic line above,
in my opinion
the 'tmp' at the right of the equals sign is always 'undefined'
so the 'fragment.appendChild(context.createElement("div"))' is always exec and return to the left 'tmp'
so the part ‘tmp ||’ isunneccessary.

any one would give me some suggestion on this problem ?

[Narretz]

Do you think this causes problems in the code? Or do you suggest we remove it? The first thing you can do is to check if all the tests pass if you remove the second statement. You can also look in the git history and see when / why this line was introduced and try to see if it is still needed

[deplay]

@Narretz
I trace the suspicious line and find that it is commited at
ddb8081

in that version of code ,at line 206,it has another problem ,there is a variable 'elem' which is unused

the 'elem' is remove in commit
8f10dca
at line 172

in a conclusion,I think the suspicious line is a little part of big snippet for internal using,has no influence on the other code.the part ‘tmp ||’ should be removed.

I am sorry I can't offer the test results becauseI have trouble in unit test ,my chrome disconnect when I run 'grunt test:unit'

Chrome 51.0.2700 (Windows 7 0.0.0): Executed 3776 of 5370 SUCCESS (0 secs / 13.8
Chrome 51.0.2700 (Windows 7 0.0.0): Executed 3777 of 5370 SUCCESS (0 secs / 13.8
9 secs)
Chrome 51.0.2700 (Windows 7 0.0.0): Executed 3777 of 5370 SUCCESS (0 secs / 13.8
9 secs)
Chrome 51.0.2700 (Windows 7 0.0.0): Executed 3777 of 5370 SUCCESS (0 secs / 13.8
Chrome 51.0.2700 (Windows 7 0.0.0): Executed 3777 of 5370 DISCONNECTED (24.576 s
ecs / 13.89 secs)
Chrome 51.0.2700 (Windows 7 0.0.0): Executed 3777 of 5370 DISCONNECTED (24.576 s
ecs / 13.89 secs)
Chrome 51.0.2700 (Windows 7 0.0.0): Executed 3777 of 5370 DISCONNECTED (24.576 s
ecs / 13.89 secs)
Warning: Karma test(s) failed. Exit code: 1 Use --force to continue.

Aborted due to warnings.

I wish someone may test it and report the result.

[gkalpak]

Yeah, it is unnecessary. @deplay, would you like to submit a PR.
(Don't worry about the tests. If you can't run them locally, they will run on CI, once you submit the PR.)

[deplay]

@gkalpak
yes, I will submit a pr asap.

[Narretz]

Closed by 2e0e77e