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

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

Open
justingrant opened this issue Feb 5, 2020 · 1 comment
Labels
kind: feature New feature or request PR welcome scope: upstream Issue in upstream dependency

Comments

@justingrant
Copy link

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:

  • devs will see build-time warnings and/or runtime console warnings (from tools like source-map-loader, create-react-app, sentry, etc.). It's often hard to silence these warnings.
  • VSCode can't set breakpoints in original library source, and in worst cases may end up showing the wrong file in the debugger.
  • In VSCode, Go To Definition won't navigate to original library source.

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) Remove /src from .npmignore (for debugging) floating-ui/floating-ui#761, Include src/ files in npm release (for resolving source maps) aws-amplify/amplify-js#2680
  2. the files setting is present in package.json without including the src folder in the list of files to publish. Example: build: add sourcemaps remix-run/react-router#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 sources paths are incorrect in declarationMap files (*.d.ts.map) emitted by tsdx #479, and it's also very common in monorepos or any project where build-time and install-time folder structure is different. Example: build: fix sourcemaps alampros/react-confetti#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.
    • Warning text idea: This package includes sourcemaps but src is excluded from publishing.
      Please add src to your package.json's files list.
    • micromatch offers a single method (the default export or micromatch.not) that will test src against the array of glob patterns found in files.
  4. If there's an .npmignore, and if it excludes src, then show a warning.
    • Warning text idea: This package includes sourcemaps but src is excluded from publishing. Please remove src from .npmignore.
    • The ignore package offers a single method that can test src against an entire .npmignore file's contents.
  5. Now iterate through all sources arrays in all (dist/**/*.map) sourcemaps and declaration maps. For each sources entry, resolve it to an absolute path using dist as the base folder and then verify that the fiile exists on disk. If it doesn't, show a warning.
    • Warning text idea: Sourcemap ./dist/index.esm.js.map points to src/index.ts which resolves to ./dist/src/index.ts. This file does not exist. Usually this problem is caused by bugs in build tools and/or invalid build configuration.
    • Replace "Sourcemap" with "Declaration map" in the message above if the filename ends with .d.ts.map

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
Copy link
Collaborator

agilgur5 commented Feb 5, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: feature New feature or request PR welcome scope: upstream Issue in upstream dependency
Projects
None yet
Development

No branches or pull requests

2 participants