Skip to content
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

Still a "NotFoundError: Failed to execute 'removeChild' on 'Node'" #14740

Closed
marco-gagliardi opened this issue Jan 31, 2019 · 6 comments
Closed

Comments

@marco-gagliardi
Copy link

marco-gagliardi commented Jan 31, 2019

I smashed again the error "NotFoundError: Failed to execute 'removeChild' on 'Node'" too.

After reading tons of threads about it I realized it's a pretty common error, however I couldn't figure out how all the provided solutions could apply to my case: I neither do DOM mutations by hand nor swallow exceptions.

I think I can reduce the relevant parts of the code as follows:

SearchDomains.js

 handleAddToCart = domain => {
    if (!domain) {
      return false
    }

    return this.props.addToCart(domain)
      .catch(e => {
        const errorCode = get(e, 'data.code')

        if (errorCode === 'DomainTaken') {
          this.setState({
            domains: {
              ...this.state.domains,
              [domain.suggestion]: {
                ...this.state.domains[domain.suggestion],
                availability: STATUS_UNAVAILABLE
              }
            },
            showPremiumDomainModalWarning: true
          })
        } else {
          this.openErrorModal()
        }

        return Promise.reject(e)
      })
  }

render () {
    const domains = this.getDomains()
    const visibleDomains = this.getVisibleDomains()

    return !this.state.isLoading && (
      <Fragment>
        <LoadMore
          isLoading={this.state.isSearching}
          total={domains.length}
          count={visibleDomains.length}
          loadingLabel='Searching, please wait...'
          infinite
          onLoadMore={this.handleLoadMore}
        >
          {React.cloneElement(this.props.children, {
            domains: this.getDomains(),
            visibleDomains: this.getVisibleDomains(),
            allVisibleDomains: this.getVisibleDomains(0, false),
            onChangeFilter: this.handleFilters,
            onChangeFormFilter: this.handleSort,
            onSearchChange: this.handleSearchChange,
            onAddToCart: this.handleAddToCart,
            ...omit(this.state, 'domains')
          })}
        </LoadMore>
        {this.state.showTimeoutAlert && this.renderTimeoutAlert()}
        {this.state.showErrorModal &&
          <CartResourcesLimitExceeded
            type='domain'
            onClose={this.closeErrorModal}
          />
        }
        <Popup
          isOpen={this.state.showPremiumDomainModalWarning}
          onClose={this.closePremiumModal}
          icon={{ name: 'ic-warning', size: 'md', color: '#f6852f' }}
          title='This is a premium domain'
          message={
            <span>Premium domain names are not registrable via Rebrandly at the moment.
              <A href='https://support.rebrandly.com/hc/en-us/articles/224626587' size='lg' target='_blank'> Read more</A>
            </span>
          }
          primaryButtonProps={{ label: 'Select another domain', onClick: this.closePremiumModal }}
        />
      </Fragment>
    )
  }

DomainItem.js:

render () {
    let discount

    if (this.props.isCoupon) {
      discount = <Text className='Text--Disable line-through m-r-24'>${this.props.renewalPrice}</Text>
    } else {
      discount = this.props.currentPrice < this.props.renewalPrice
        ? <Text size='x-small' className='Text--Disable m-r-4'>
          <span className='line-through m-r-4'>
            ${this.props.renewalPrice}
          </span>
          NOW
        </Text>
        : <Text size='x-small' className='Text--Disable m-r-4'>
          ONLY
        </Text>
    }

    return (
      <Fragment>
        {this.props.availability === STATUS_AVAILABLE
          ? <Fragment>
            {discount}
            {!this.props.isCoupon &&
              <Tooltip
                overlay={<span>First year, then ${this.props.renewalPrice} renewal. <RouterLink target='_blank' href='https://support.rebrandly.com/hc/en-us/articles/225551448-Domain-#-for-Custom-Short-Urls'>Learn more</RouterLink></span>}
                placement='top'
              >
                <Text className='DomainItem__azure m-r-24'>${this.props.currentPrice}</Text>
              </Tooltip>
            }
            {this.state.isLoading
              ? <Loading size='md' />
              : <Button
                isLoading={this.state.isLoading}
                {...this.props.actionButtonProps}
                onClick={this.handleOnClick}
              />
            }
          </Fragment>
          : <Text className='DomainItem__unavailable Text--SubDetail' size='small'>
            {!!this.props.availability && (this.props.availability === STATUS_UNKNOWN ? 'Couldn’t check domain details' : 'Not available')}
          </Text>
        }
      </Fragment>
    )
  }

I was pretty sure there had to be some messed (async) logic changing the cards under the hood between a render cycle and the next one, so I spent some time trying to debug all the updates, looking for the reason that was driving to an inconsistency in the DOM reconciliation.

Indeed, I figured out the issue was with that ternary operator in the DomainItem's render function: dropping temporarily the "else" case, thus, preventing react from removing a content and injecting a different one, switched off the error. Of course this was not an acceptable fix.

Then, I realized that everything could be solved by just removing the wrapping <Fragment> in the return statement of DomainItem's render function. Now everything works fine:

return this.props.availability === STATUS_AVAILABLE
     ? <Fragment>
       {discount}
       {!this.props.isCoupon &&
       <Tooltip
         overlay={<span>First year, then ${this.props.renewalPrice} renewal. <RouterLink target='_blank' href='https://support.rebrandly.com/hc/en-us/articles/225551448-Domain-#-for-Custom-Short-Urls'>Learn more</RouterLink></span>}
         placement='top'
       >
         <Text className='DomainItem__azure m-r-24'>${this.props.currentPrice}</Text>
       </Tooltip>
       }
       {this.state.isLoading
         ? <Loading size='md' />
         : <Button
           isLoading={this.state.isLoading}
           {...this.props.actionButtonProps}
           onClick={this.handleOnClick}
         />
       }
     </Fragment>
     : <Text className='DomainItem__unavailable Text--SubDetail' size='small'>
       {!!this.props.availability && (this.props.availability === STATUS_UNKNOWN ? 'Couldn’t check domain details' : 'Not available')}
     </Text>

Didn't know if this is an intended behaviour so I thought it was worthy to report it.

@gaearon
Copy link
Collaborator

gaearon commented Feb 8, 2019

Sorry, this is a large blob of code that references your custom components and so it doesn't really tell us much.

If you narrow it down to a reproducible case we'd be happy to take a look. Thanks.

@gaearon gaearon closed this as completed Feb 8, 2019
@marco-gagliardi
Copy link
Author

marco-gagliardi commented Feb 8, 2019

Hey @gaearon sure. I think the relevant part is that this pattern:

return (
      <Fragment>
        {condition
          ? <Fragment>
            ...
          </Fragment>
          :  ...
        }
      </Fragment>
    )

generates a NotFoundError to me when condition value changes. Solved by removing the wrapping Fragment:

return condition
          ? <Fragment>
            ...
          </Fragment>
          :  ...

@gaearon
Copy link
Collaborator

gaearon commented Feb 8, 2019

Can you try to create a minimal isolated reproducing example?

@gaearon
Copy link
Collaborator

gaearon commented Feb 11, 2019

I guess this might be the same issue as #14811 and would be fixed by #14820. Are you rendering an empty portal by any chance?

@gaearon
Copy link
Collaborator

gaearon commented Feb 14, 2019

This is probably fixed in 16.8.2. If not please file a new issue with a reproducing case.

@yevmoroz
Copy link

yevmoroz commented Nov 4, 2019

@gaearon do you think following #17218 can be related?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants