jaredpalmer / tsdx

Zero-config CLI for TypeScript package development
https://tsdx.io
MIT License
11.28k stars 508 forks source link

Emit a warning if sourcemaps point to files that won't exist when devs install the package #494

Open justingrant opened 4 years ago

justingrant commented 4 years ago

Current Behavior

For the last year I've been helping libraries fix their sourcemaps. One thing I learned from this weird hobby is that there are a few common issues that cause most invalid sourcemaps. The two big issues I saw (and where I think tools like tsdx could help) were:

Either of these sourcemap issues will cause the following problems for developers who install the library:

Desired Behavior

tsdx would warn during the build process if sourcemaps point to invalid paths or files that don't exist in the package published to npm.

There are (at least) three reasons this can happen: 1) the src folder is excluded using .npmignore. Examples: (of libraries that had this problem) https://github.com/popperjs/popper-core/pull/761, https://github.com/aws-amplify/amplify-js/pull/2680 2) the files setting is present in package.json without including the src folder in the list of files to publish. Example: https://github.com/ReactTraining/react-router/pull/6823 3) the paths inside the sourcemap don't correspond to real paths relative to dist. This is most commonly caused when bugs in build tools or bad build config that causes emitting bad relative paths into the map's sources. An example of this kind of problem is #479, and it's also very common in monorepos or any project where build-time and install-time folder structure is different. Example: https://github.com/alampros/react-confetti/pull/79

This behavior should only apply to builds that actually create sourcemaps. If sourcemaps are disabled and no .map files are generated, then this feature should be a no-op.

There should be a way to opt out (in tsdx config) of these checks.

Suggested Solution

I won't have time in the short term so sadly I can't commit to PR this. But I did think through a possible solution and I captured some ideas below in case this might be helpful to anyone picking this up later.

1) Run some checking code at the end of the build, after all sourcemap and declaration map files have been generated. 2) If there's no files in dist/**/*.map, stop. No sourcemaps, no problem! 3) If there's a files section in package.json, is the src folder in it? If not, show a warning.

In theory, (5) could be merged with (3) and (4) by testing every sources entry against .npmignore and files, instead of only checking one src folder. This every-file approach might catch more corner cases, e.g. if each source file is individually added to files or .npmignore. But I've never seen those corner cases in the wild. So IMHO the full-folder checks for .npmignore and files are probably good enough.

Does tsdx allow user-editable src and dist? If yes, then it'd make the solution more complicated.

Who does this impact? Who is this for?

Library authors who want to help their users debug more easily, esp. in VS Code.

Describe alternatives you've considered

My current alternative is to file issues and PRs against OSS libraries to help get sourcemaps fixed. Doing this repeatedly is getting old!

Additional context

A side effect of adding this feature would be that all tests that currently generate sourcemaps would also start validating those sourcemaps and would emit warnings if validation fails. This might prevent more bugs like #479 by making future sourcemap issues easier to catch via test automation.

agilgur5 commented 4 years ago

I think this is a good idea for TSDX to have, but this should definitely be a standalone package that TSDX could import. By your own examples, that sounds like something that would be useful to the community outside of TSDX and then you or anyone can run standalone to automatically check any package/repo for sourcemap issues