-
Notifications
You must be signed in to change notification settings - Fork 773
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
Include original .ts files in the NPM packages to allow proper debugging #1287
Comments
hey, thanks, yes we've had this discussion before in ethereumjs/rlp#97 i'd be in favor of adding the source files since it can be useful in certain situations. |
Yes, I guess I would be in favor too, so maybe let's just right out do this. Would be nice to hear the opinions from @acolytec3 and @jochem-brouwer too and see if there are major reservations for whatever reasons. @gabmontes I can't promise you to directly do version bumps (time constraints) on all the libraries with this (if we agree to do), are there any libraries which are most pressing to you and you would very much want to see released? |
@ryanio @holgerd77 thanks for your quick response! Not in a hurry right now but it would help others that may need debugging in the future. Thanks again!!! |
I don't have any concerns with it (though then again I've not encountered
this specific sort of issue previously so have no past experience. Seems
like it would make sense to do. If people start complaining about package
sizes, we could always do some sort of minified version of each that strips
the sources back out.
…On Tue, Jun 8, 2021 at 8:18 PM Gabriel Montes ***@***.***> wrote:
@ryanio <https://github.com/ryanio> @holgerd77
<https://github.com/holgerd77> thanks for your quick response!
Not in a hurry right now but it would help others that may need debugging
in the future.
Thanks again!!!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1287 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEENFXEDVP3QKQZDTKOOI3TTR2XOFANCNFSM46IPDEUA>
.
|
@gabmontes I would like to give this a try but not really sure how to approach. Is there a simple way to get into this setup context with having one of the libraries available in the browser and using Chrome for debugging? This would generally be pretty helpful for us to have a quick way to do some occasional (manual) test runs apart from the Karma runs in the browser, I guess all of us have little experience with this and it would be nice to get some better feeling for that (I personally have used ChromeDevTools on other occasions though). (Once we've got that it would be good to document). I actually tried to use our run-code-browser example from the VM, but interestingly enough the |
@holgerd77 this is no related to running the code in the browser. In fact I saw the issue while debugging code running in Node.js, using the Chrome DevTools. |
Ok, I was finally able to recreate this respectively do a relevant setup by using this tutorial to set up (or just: using) Chrome for doing Node.js debugging, here is some screenshot for illustration: Some note on this: For this to use one has to first add the folder one is working with to the workspace ("Filesystem" tab in the dev tools) and then one can set a breakpoint in the code. Otherwise code runs triggered from the console will just terminate and files won't be available for inspection. Then you can run I then tested on our root problem here (the missing source files) by mouse-hovering on e.g. Same debug test in the Chrome DevTools now lead to going to the Yeah, I would say proven to be both useful and working, so I would say we can add. 🤩 Will directly open a PR. |
The published package in NPM only contains the
/dist/*
files but when debugging with i.e. the Chrome DevTools and stepping into the library, an error is thrown because the original/src/*.ts
files cannot be found and therefore the sourcemaps can't be used to display the proper source lines being executed.Please update the NPM package to also include the
/src
directory.Other packages distributing transpiled TypeScript files have the same issue. See MetaMask/eth-sig-util#142.
The text was updated successfully, but these errors were encountered: