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

Add 'src' to package.json files in templates to improve source maps #252

Closed
wants to merge 1 commit into from
Closed

Add 'src' to package.json files in templates to improve source maps #252

wants to merge 1 commit into from

Conversation

yordis
Copy link
Contributor

@yordis yordis commented Oct 15, 2019

Since source maps point to ../src/* if people don't publish the src folder, source maps are not that useful.

@yordis
Copy link
Contributor Author

yordis commented Oct 15, 2019

Actually I think I am wrong on this one, it seems that the source map includes all source code content as well.

@jaredpalmer can you confirm this?

@jaredpalmer
Copy link
Owner

I think sourcemaps work fine (formik uses them)

@jaredpalmer
Copy link
Owner

but maybe not since the rollup-plugin-ts update?

@yordis
Copy link
Contributor Author

yordis commented Oct 15, 2019

Nah I think I made a mistake 😅 sorry

@yordis yordis closed this Oct 15, 2019
@yordis yordis deleted the yordis/improve-package-config branch October 15, 2019 16:20
@agilgur5 agilgur5 changed the title Improve package json template Add 'src' to package.json files in templates Mar 17, 2020
@agilgur5 agilgur5 changed the title Add 'src' to package.json files in templates Add 'src' to package.json files in templates to improve source maps Mar 17, 2020
@agilgur5
Copy link
Collaborator

Seems very related to #494 -- @justingrant could you elaborate here on why src would be necessary to include for source maps if the .map file includes the contents of the source already?

Based on your issues in other places, is this just to be able to add breakpoints in the library source code when debugging? Or is it necessary for "Peek" / "Go to Definition" too? And maybe other things I'm missing?

@justingrant
Copy link

justingrant commented Mar 17, 2020

@agilgur5 - here's a few reasons why including src is helpful, especially for IDE users:

  • You can set a breakpoint before starting a debug session in your IDE. This is helpful for debugging startup issues. I also do this sometimes whenever I add a new dependency where I want to step through it in the debugger the very first time I use it so I can make sure that I'm using it properly. By setting a breakpoint immediately after I npm install the library, it ensures that I remember to debug through it even if the first time I actually debug that code isn't until a week later.
  • You can use the IDE's file-search features to find the source of specific methods or code in the library source (and optionally set a breakpoint there) without having to shell out to GitHub to do the search there, and then to manually correlate what you see in GitHub to the library source.
  • Apps that use webpack to bundle libraries which have ghost sources entries (where those source files don't exist on disk) can cause VSCode may load the wrong source file in the debugger. This can happen when a sourcemap contains a relative path in a sources entry (e.g. ./src/index.ts) that also exists in the app. VSCode tries multiple base paths to convert an entry in sources to a full path on disk, and if you get a match (even a false positive match) then VSCode will never even load the inline source.
  • Some build tools will report a warning when a source file is present in sources but not on disk.
  • [EDIT] Go To Declaration using declaration maps might also require original source files. Not sure about this one though-- would need to test it.

@agilgur5
Copy link
Collaborator

Thanks for the great write-up @justingrant ! I didn't know about the VSCode base paths Issue 😰
Does "Peek"/"Go to Definition" work without the source? I believe it works a little differently when you have the source but I'm not totally sure.

In any case, I'll re-open and merge this PR. Or rather, apparently can't re-open as the repo/branch doesn't exist any more on the fork, so I'll have to open a new PR

@justingrant
Copy link

Does "Peek"/"Go to Definition" work without the source? I believe it works a little differently when you have the source but I'm not totally sure.

@agilgur5 - my understanding is that Go to Definition will navigate to a library's original source if: a) there's a valid declaration map (.d.ts.map); AND b) the original source files that the declaration map refers to are present on disk.

That said I've been told on another thread that folks can't get declaration maps working at all (even if the conditions above are satisfied) so there may be some other problem going on.

Regardless, I strongly suspect that Go To Definition won't go to original source if the original source files are missing, because AFAIK the IDE won't load the source in the sourcemap file unless the debugger is running.

@agilgur5
Copy link
Collaborator

Gotcha. Thanks for filling in the knowledge gaps!

I opened and merged #620 (while crediting OP here) to add src to the default package.json.files 👍

@allcontributors please add @justingrant for questions

@allcontributors

This comment has been minimized.

@agilgur5
Copy link
Collaborator

agilgur5 commented Mar 17, 2020

@allcontributors please add @justingrant for questions

it seems to fail occasionally if you only mention one type of contribution 🤷‍♂

@allcontributors
Copy link
Contributor

@agilgur5

I've put up a pull request to add @justingrant! 🎉

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

Successfully merging this pull request may close these issues.

4 participants