ericclemmons / click-to-component

Option+Click React components in your browser to instantly open the source in VS Code
MIT License
1.94k stars 75 forks source link

Try find parent when couldn't find a React instance ( has source) for the element #34

Closed xxleyi closed 11 months ago

xxleyi commented 2 years ago
export function getSourceForElement(
  /**
   * @type {HTMLElement}
   */
  element
) {
  const instance = getReactInstanceForElement(element)
  const source = getSourceForInstance(instance)

  if (source) return source

  console.warn("Couldn't find a React instance for the element", element)
}

Now in getSourceForElement, when we can't find source of an element, we just return undefined.

Can we try to find a parent React instance which has a source?

moroshko commented 2 years ago

Potentially related, I'm getting an error when Option+clicking an element:

Screen Shot 2022-04-28 at 5 15 23 pm
Couldn't find a React instance for the element <div data-click-to-component-target=​"HOVER" style=​"position:​ fixed;​ z-index:​ 9999;​ inset:​ 16px;​ pointer-events:​ none;​">​</div>​
getSourceForElement @ getSourceForElement.js?be65:19
handleClick @ ClickToComponent.js?6fa9:42
getPathToSource.js?2ef7:12 Uncaught TypeError: Cannot read properties of undefined (reading 'columnNumber')
    at getPathToSource (getPathToSource.js?2ef7:12:1)
    at handleClick (ClickToComponent.js?6fa9:43:37)
xxleyi commented 2 years ago

Potentially related, I'm getting an error when Option+clicking an element:

Screen Shot 2022-04-28 at 5 15 23 pm
Couldn't find a React instance for the element <div data-click-to-component-target=​"HOVER" style=​"position:​ fixed;​ z-index:​ 9999;​ inset:​ 16px;​ pointer-events:​ none;​">​</div>​
getSourceForElement @ getSourceForElement.js?be65:19
handleClick @ ClickToComponent.js?6fa9:42
getPathToSource.js?2ef7:12 Uncaught TypeError: Cannot read properties of undefined (reading 'columnNumber')
    at getPathToSource (getPathToSource.js?2ef7:12:1)
    at handleClick (ClickToComponent.js?6fa9:43:37)

Yes, I also met this error. I have patched this feature in my project

ericclemmons commented 2 years ago

Oh! Interestingly I used to have logic for this, but couldn't reproduce it:

const getReactInstancesForElement = (element: HTMLElement) => {
  const instances: Array<Fiber> = [];
  let instance = getReactInstanceFromElement(element);

  // Find the "source of truth" for the current DOM node
  while (instance) {
    instances.push(instance);

    instance = instance._debugOwner;
  }

  // `_debugOwner` seems to stop when there's a `_debugSource`, so recursively traverse up the DOM for another Fiber node
  if (element.parentElement) {
    instances.push(...getReactInstancesFromElement(element.parentElement));
  }

  return instances;
};

I removed it because I was getting duplicate instances since _debugOwner can often lead to the same nodes.

@xxleyi & @moroshko – Can y'all fork and update one of the https://github.com/ericclemmons/click-to-component/tree/main/apps with sample code to reproduce?

xxleyi commented 2 years ago

@ericclemmons Oh, I think I didn't express clearly. What I mean is find React instance ancestor, not html element parent.

What I am doing is like this:

export function getSourceForElement(
  /**
   * @type {HTMLElement}
   */
  element
) {
  const instance = getReactInstanceForElement(element);
  const source = getSourceForInstance(instance);

  if (source) return source;

  console.warn("Couldn't find a React instance for the element", element);
  console.info('Let us try to find a React instance ancestor which has a source');
  const instances = getReactInstancesForElement(element);
  for (const ins of instances) {
    // eslint-disable-next-line @typescript-eslint/no-shadow
    const source = getSourceForInstance(ins);
    if (source) return source;
  }
}

In my project, it seems that only those component in my src have _source annotation, those component in third packages don't have _source annotation. This behavor looks good to me, because when I want to jump to source code, I really don't care about third package.

ericclemmons commented 2 years ago

Ah, good point! If Babel isn't transpiling a 3rd party lib's JSX (because it's already React.createElement), then there won't be any __source.

This looks like an easy fix that I should be able to reproduce, too!

ma101an commented 2 years ago

@ericclemmons correct me if I'm wrong, it seems like you would need to find the closest parent element with __source not null.

PS: I've been looking forward to use this in our Application. But this bug is being the blocker for it.

LeonMueller-OneAndOnly commented 2 years ago

Hello folks, i have found a solution four your problem. I implemented @ma101an's solution. When no valid source is found the next parent element will be used as a fallback an so on until a valid source is found.

I forked the codebase to TypeScript in order to debug in my application, therefore i will provide the source code directly. I modifed src/getSourceForElement slightly:

import { getReactInstanceForElement } from "./getReactInstanceForElement"
import { getSourceForInstance } from "./getSourceForInstance"

/**
 * @typedef {import('react-reconciler').Fiber} Fiber
 */

export function getSourceForElement(element: HTMLElement) {
  const instance = getReactInstanceForElement(element)
  const source = getSourceForInstance(instance)

  if (source) return source

  // console.warn("Couldn't find a React instance for the element", element)
  // console.info("Let us try to find a React instance ancestor which has a source")

  const fallbackSource = getFirstParentElementWithSource(element)
  return fallbackSource
}

function getFirstParentElementWithSource(element: HTMLElement): any {
  const parentElement = element.parentElement
  if (parentElement === null) throw new Error("No parent found")

  const instance = getReactInstanceForElement(parentElement)
  const source = getSourceForInstance(instance)

  if (source) return source
  else return getFirstParentElementWithSource(element)
}
nivnahmias commented 1 year ago

hey @ericclemmons , any chance to integrate @LeonMueller-OneAndOnly 's solution and release a fix? I really want to use this with my team. Do you want me to fork and create a PR?

ericclemmons commented 1 year ago

Sorry for the delay. Life stuff came up.

I've added leonelmeque, @nivnahmias, & @LeonMueller-OneAndOnly as Collaborators.

Feel free to merge what you need! I trust y'all :)

Screen Shot 2023-01-10 at 4 09 01 PM

spacecat commented 1 year ago

Hi, I got the "Couldn't find a React instance for the element" warning in Chrome DevTools today when installing this plugin for the first time and doing a Option + Left Click.

And under the warning there's an error in the DevTools console:

Uncaught TypeError: Cannot read properties of undefined (reading 'columnNumber') at getPathToSource (getPathToSource.js:12:1) at handleClick (ClickToComponent.js:43:1) getPathToSource @ getPathToSource.js:12 handleClick @ ClickToComponent.js:43

I was just wondering what the status is? It sounded like there was a fix on the way? :)

LeonMueller-OneAndOnly commented 11 months ago

Hello, Following pull-request was merged to main, in order to fix this issue: https://github.com/ericclemmons/click-to-component/pull/79