Skip to content

Remove unused export from DOMChildrenOperations #6301

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Mar 20, 2016
Merged

Remove unused export from DOMChildrenOperations #6301

merged 1 commit into from
Mar 20, 2016

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Mar 19, 2016

It appears that the only usage of DOMChildrenOperations.updateTextContent() was removed in #5753. More specifically, #5753 started using replaceDelimitedText() instead, which it added in the same commit.

setTextContent() itself is still used inside DOMChildrenOperations.processUpdates()—it’s just that there is no need to re-export it from DOMChildrenOperations now.

Due Diligence

No DOMChildrenOperations.updateTextContent calls in the codebase:

Searching 541 files for "DOMChildrenOperations." (case sensitive)

/Users/dan/p/react/src/renderers/dom/client/ReactDOMIDOperations.js:
   29    dangerouslyProcessChildrenUpdates: function(parentInst, updates) {
   30      var node = ReactDOMComponentTree.getNodeFromInstance(parentInst);
   31:     DOMChildrenOperations.processUpdates(node, updates);
   32    },
   33  };

/Users/dan/p/react/src/renderers/dom/shared/ReactComponentBrowserEnvironment.js:
   27  
   28    replaceNodeWithMarkup:
   29:     DOMChildrenOperations.dangerouslyReplaceNodeWithMarkup,
   30  
   31    /**

/Users/dan/p/react/src/renderers/dom/shared/ReactDOMTextComponent.js:
  137          this._stringText = nextStringText;
  138          var commentNodes = this.getNativeNode();
  139:         DOMChildrenOperations.replaceDelimitedText(
  140            commentNodes[0],
  141            commentNodes[1],

3 matches across 3 files

Existing matches of updateTextContent refer to a function defined on ReactMultiChild rather than the one exported from DOMChildrenOperations:

Searching 541 files for "updateTextContent" (case sensitive)

/Users/dan/p/react/src/renderers/dom/shared/ReactDOMComponent.js:
  976        this.updateChildren(null, transaction, context);
  977      } else if (lastHasContentOrHtml && !nextHasContentOrHtml) {
  978:       this.updateTextContent('');
  979      }
  980  
  981      if (nextContent != null) {
  982        if (lastContent !== nextContent) {
  983:         this.updateTextContent('' + nextContent);
  984        }
  985      } else if (nextHtml != null) {

/Users/dan/p/react/src/renderers/shared/reconciler/ReactMultiChild.js:
  240       * @internal
  241       */
  242:     updateTextContent: function(nextContent) {
  243        var prevChildren = this._renderedChildren;
  244        // Remove any rendered children.
  ...
  246        for (var name in prevChildren) {
  247          if (prevChildren.hasOwnProperty(name)) {
  248:           invariant(false, 'updateTextContent called on non-empty component.');
  249          }
  250        }
  ...
  266        for (var name in prevChildren) {
  267          if (prevChildren.hasOwnProperty(name)) {
  268:           invariant(false, 'updateTextContent called on non-empty component.');
  269          }
  270        }

5 matches across 2 files

It appears that this is safe to remove.

Reviewers

@sebmarkbage @spicyj

@sophiebits
Copy link
Collaborator

👍

@facebook-github-bot
Copy link

@gaearon updated the pull request.

gaearon added a commit that referenced this pull request Mar 20, 2016
Remove unused export from DOMChildrenOperations
@gaearon gaearon merged commit 441d9b6 into facebook:master Mar 20, 2016
@gaearon gaearon deleted the remove-dead-export branch March 20, 2016 00:41
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants