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

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-Pricing-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-Pricing-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?

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

No branches or pull requests

3 participants