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

Bug: fix BigInt in copyElementPath in react-devtools #17895

Closed
leidegre opened this issue Jan 23, 2020 · 7 comments · Fixed by #17931
Closed

Bug: fix BigInt in copyElementPath in react-devtools #17895

leidegre opened this issue Jan 23, 2020 · 7 comments · Fixed by #17931

Comments

@leidegre
Copy link
Contributor

leidegre commented Jan 23, 2020

This is a continuation of an previous issue to add support for the BigInt data type in React DevTools.

Original PR #17233 (merged)

This happens when you try to copy a BigInt value to clipboard via React DevTools.

Would @nutboltu mind taking a look?

backend.js:1 Uncaught TypeError: Do not know how to serialize a BigInt
    at JSON.stringify (<anonymous>)
    at c (backend.js:1)
    at Object.copyElementPath (backend.js:6)
    at t.<anonymous> (backend.js:6)
    at t.r.emit (backend.js:6)
    at backend.js:32
    at t (backend.js:8)
c @ backend.js:1
copyElementPath @ backend.js:6
(anonymous) @ backend.js:6
r.emit @ backend.js:6
(anonymous) @ backend.js:32
t @ backend.js:8
postMessage (async)
(anonymous) @ contentScript.js:1
<./app-insights/app-insights>:50 Uncaught TypeError: Cannot read property 'message' of null
    at trackError (<./app-insights/app-insights>:50)
    at eval (<./app-insights/app-insights>:22)
trackError @ <./app-insights/app-insights>:50
eval @ <./app-insights/app-insights>:22
setTimeout (async)
eval @ <./app-insights/app-insights>:21
error (async)
initAppInsights @ <./app-insights/app-insights>:17
main @ VM70658 client>:101
main @ ./../../../node_modules/@tessin/tcm/lib/dev/boot-loader:31
async function (async)
main @ ./../../../node_modules/@tessin/tcm/lib/dev/boot-loader:27
(anonymous) @ 219:3435
backend.js:1 Uncaught TypeError: Do not know how to serialize a BigInt
    at JSON.stringify (<anonymous>)
    at c (backend.js:1)
    at Object.copyElementPath (backend.js:6)
    at t.<anonymous> (backend.js:6)
    at t.r.emit (backend.js:6)
    at backend.js:32
    at t (backend.js:8)
c @ backend.js:1
copyElementPath @ backend.js:6
(anonymous) @ backend.js:6
r.emit @ backend.js:6
(anonymous) @ backend.js:32
t @ backend.js:8
postMessage (async)
(anonymous) @ contentScript.js:1
<./app-insights/app-insights>:50 Uncaught TypeError: Cannot read property 'message' of null
    at trackError (<./app-insights/app-insights>:50)
    at eval (<./app-insights/app-insights>:22)
trackError @ <./app-insights/app-insights>:50
eval @ <./app-insights/app-insights>:22
setTimeout (async)
eval @ <./app-insights/app-insights>:21
error (async)
initAppInsights @ <./app-insights/app-insights>:17
main @ VM70658 client>:101
main @ ./../../../node_modules/@tessin/tcm/lib/dev/boot-loader:31
async function (async)
main @ ./../../../node_modules/@tessin/tcm/lib/dev/boot-loader:27
(anonymous) @ 219:3435
backend.js:1 Uncaught TypeError: Do not know how to serialize a BigInt
    at JSON.stringify (<anonymous>)
    at c (backend.js:1)
    at Object.copyElementPath (backend.js:6)
    at t.<anonymous> (backend.js:6)
    at t.r.emit (backend.js:6)
    at backend.js:32
    at t (backend.js:8)
c @ backend.js:1
copyElementPath @ backend.js:6
(anonymous) @ backend.js:6
r.emit @ backend.js:6
(anonymous) @ backend.js:32
t @ backend.js:8
postMessage (async)
(anonymous) @ contentScript.js:1
<./app-insights/app-insights>:50 Uncaught TypeError: Cannot read property 'message' of null
    at trackError (<./app-insights/app-insights>:50)
    at eval (<./app-insights/app-insights>:22)
@leidegre leidegre added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jan 23, 2020
@bvaughn bvaughn added Component: Developer Tools good first issue Type: Bug and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Jan 23, 2020
@nutboltu
Copy link
Contributor

@leidegre I will take a look.

@bvaughn
Copy link
Contributor

bvaughn commented Jan 23, 2020

@nutboltu: This issue is all yours! 😄

I've added the "good first issue (taken)" label so that others will know not to start work on the issue. If you change your mind about the issue, no worries! Just let me know so that I can remove the label and free it up for someone else to claim.

Cheers!

@bvaughn
Copy link
Contributor

bvaughn commented Jan 29, 2020

Hey @nutboltu how's this task coming along? :)

@nutboltu
Copy link
Contributor

@bvaughn Raised a PR. #17931 . I need to test it in local properly. currently having issue with the devtools-extension in my local machine.

@bvaughn
Copy link
Contributor

bvaughn commented Jan 30, 2020

You should be able to test this using the testing harness app:
https://github.com/facebook/react/tree/master/packages/react-devtools-shell

Might be easier. Just add a BigInt prop to UnserializableProps for exampe.

@nutboltu
Copy link
Contributor

react-devtools-shell is really handy to test :)

@bvaughn
Copy link
Contributor

bvaughn commented Jan 31, 2020

Yes. It's what I typically use when building a new feature. Then I just do a final test in the actual extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants