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.

Sign up for GitHub

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

Jump to bottom

[DevTools Bug]: Hook names fail to parse if source file for Component is an html file #22319

Open
jstejada opened this issue Sep 14, 2021 · 6 comments
Open

[DevTools Bug]: Hook names fail to parse if source file for Component is an html file #22319

jstejada opened this issue Sep 14, 2021 · 6 comments
Assignees
@jstejada
Labels
Component: Developer Tools Type: Bug

Comments

@jstejada
Copy link
Contributor

jstejada commented Sep 14, 2021 •
edited
Loading

When testing our fixture in Safari (./fixtures/devtools/standalone/index.html) with the standalone build if DevTools, I noticed that we were unable to parse hook names because the source file where the components are defined isn't a JavaScript file, and instead an HTML file (as in index.html), which contains HTML code, and some JS inside script tags, so the babel parser fails to parse the file.

Specifically:

// parseSourceAndMetadata.js

const originalSourceAST = withSyncPerfMeasurements(
  '[@babel/parser] parse(originalSourceCode)',
  () =>
    parse(originalSourceCode, {
      sourceType: 'unambiguous',
      plugins: ['jsx', plugin],
    }),
);

In the above code, the call to parse fails because originalSourceCode is actually html (with JS inside) like:

<!DOCTYPE html>
<html>
  <head>
    <meta charset="UTF-8" />
    <title>TODO List</title>

    <!-- ... -->

  <body>
    <div id="root"></div>
    <script type="text/javascript">
      const { useState } = React;

      function App() {
        const [count, setCount] = useState(0);
        return null;
      }

      ReactDOM.render(React.createElement(App), document.getElementById("root"));
    </script>
  </body>

And parse expects actual JS code.

DevTools should handle the case when source files aren't JS files, and still be able to extract out the hook names, or fail more gracefully. As of #22320, we log to the console a slightly better error message when this happens, as opposed to an obscure parsing error.

Repro steps

  1. Run standalone app
  2. Open fixtures file in safari: ./fixtures/devtools/standalone/index.html
  3. Try getting hook names, observe that parsing source files fails
@jstejada jstejada added Type: Bug Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Component: Developer Tools and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Sep 14, 2021
@jstejada jstejada self-assigned this Sep 14, 2021
@jstejada jstejada mentioned this issue Sep 14, 2021
Merged
@bvaughn
Copy link
Contributor

bvaughn commented Sep 15, 2021

I think the title of this issue might be misleading. (I might be wrong but) I don't think the cause is an inline <script type="text/javascript"> tag but rather the fact that our demo uses <script type="text/babel"> which has non-standard JS syntax (JSX) that requires a special plug-in to parse.

We could look into how much weight this additional plug-in would add to the bundle, but initially I lean towards saying that should just explicitly not support JSX syntax.

Again though, I could be wrong and maybe it really is the inline tag part. 😄

@jstejada jstejada changed the title [DevTools Bug]: Hook names fail to parse if source JS is defined in an htm file with inline script tags [DevTools Bug]: Hook names fail to parse if source JS is defined in an html file with inline script tags Sep 15, 2021
@jstejada jstejada changed the title [DevTools Bug]: Hook names fail to parse if source JS is defined in an html file with inline script tags [DevTools Bug]: Hook names fail to parse if source file for javascript is an html file Sep 15, 2021
@jstejada
Copy link
Contributor Author

@bvaughn so i tested this by making the index.html file only contain a regular "text/javascript" script tag, without jsx or anything:

image

and our call to babel.parse() still fails for parse the file, I presume because parse() expects the input to be JS , but we are passing html, so it fails to parse the html outside the script tags.

not sure what the best approach would be here, maybe try to extract the JS from script tags if we detect that the source file is an html file?

@jstejada jstejada changed the title [DevTools Bug]: Hook names fail to parse if source file for javascript is an html file [DevTools Bug]: Hook names fail to parse if source file for Component is an html file Sep 15, 2021
@bvaughn
Copy link
Contributor

bvaughn commented Sep 15, 2021

The "HTML" you're referring to above is also JSX :)

React.createElement('div', {id: 'root'}; // <div id="root"

@jstejada
Copy link
Contributor Author

jstejada commented Sep 15, 2021 •
edited
Loading

we discussed offline and there was confusion about what I was referring to in this issue. I updated the summary to try to clarify the problem.

@bvaughn
Copy link
Contributor

bvaughn commented Nov 4, 2021 •
edited
Loading

As of #22320, we log to the console a slightly better error message when this happens, as opposed to an obscure parsing error

For added context, the error I'm seeing in the console is:

Error: Hook source code location not found.
    at u (hookNamesCache.js:215)
    at loadSourceAndMetadata.js:435

This happens before the parsing step. I wonder what has changed since you originally posted this?

Digging in further, that's because hookSource is missing a file name, e.g.:

hookSource: {
  columnNumber: 22,
  fileName: undefined,
  functionName: "ListItem",
  lineNumber: 131,
}

Looking at the error-stack-parser source, it seems like an undefined fileName indicates a stack frame of either "eval" or "<anonymous>". Sure enough, looking at the Error stack for one of our components defined in an HTML file like this, I see e.g. " at ListItem (<anonymous>:135:22)".

I'm not sure if there's much we can do about this.

I'm seeing this same behavior (an undefined fileName) when using both Chrome and Safari.

@akgupta0777
Copy link
Contributor

This seems easy to fix, Maybe I can fix this.
@bvaughn or @jstejada
Any other information or guidance you wanna give for a headstart.

Also, I want to know about how hook name parsing is done in react devtools. Any resources for that will be great for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jstejada jstejada

Labels
Component: Developer Tools Type: Bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@bvaughn @jstejada @akgupta0777