build: fix sourcemaps - Githubissues

justingrant commented 4 years ago

react-confetti's sourcemaps don't actually point to source files in the sourcemap's sources array. This causes a few problems:

Warnings are emitted into the console at runtime when using some tools that check for presence of source files, like source-map-loader.
Using an IDE debugger like VSCode, you can't set breakpoints on the actual source files unless the debugger is running, because the source files aren't published to npm so they don't actually exist on disk on a client dev's machine. This makes it harder to debug startup scenarios or other cases where it's helpful to proactively set breakpoints before starting the debugger.
It also makes it harder for devs who want to understand how the library works, because the code inside node_modules is confusing transpiled ES5 javascript. The only way today to see the actual TS code is to go to github, instead of being able to look inside node_modules and easily understand what the library is doing.

This PR fixes these problems by making the following changes:

Adds src/ to the files published to the npm registry
Changes the invalid paths emitted by Webpack into relative URLs that will be valid relative paths pointing to source files when a developer installs this package from the registry.

With these changes made, client devs can view and set breakpoints in the actual TS source code of this library. FWIW, this PR is analogous to similar PRs merged into aws-amplify, react-router, swiper, libphonenumber-js, etc.

Here's what the start of react-confetti.js.map looks like in the current version, with line breaks added for clarity:

{"version":3,"sources":[
  "webpack://ReactConfetti/webpack/universalModuleDefinition",
  "webpack://ReactConfetti/webpack/bootstrap",
  "webpack://ReactConfetti/./node_modules/tween-functions/index.js",
  "webpack://ReactConfetti/./src/Confetti.ts",
  "webpack://ReactConfetti/./src/Particle.ts",
  "webpack://ReactConfetti/./src/ParticleGenerator.ts",
  "webpack://ReactConfetti/./src/ReactConfetti.tsx",
  "webpack://ReactConfetti/./src/utils.ts",
  "webpack://ReactConfetti/external {\"root\":\"React\",\"commonjs2\":\"react\",\"commonjs\":\"react\",\"amd\":\"react\"}"
],

Those paths don't exist on disk, and the webpack:// prefix is not recognized by webpack itself (when this library is being bundled into an app).

Here's what it looks like after this PR:

{"version":3,"sources":[
  "webpack/universalModuleDefinition",
  "webpack/bootstrap",
  "../../tween-functions/index.js",
  "../src/Confetti.ts",
  "../src/Particle.ts",
  "../src/ParticleGenerator.ts",
  "../src/ReactConfetti.tsx",
  "../src/utils.ts",
  "external {\"root\":\"React\",\"commonjs2\":\"react\",\"commonjs\":\"react\",\"amd\":\"react\"}"
],

All the paths that point to actual source files have been updated to point to actual locations on disk where the files will be on the developer's machine after this library is installed. webpack/universalModuleDefinition, webpack/bootstrap, and external ... aren't real files, but they don't cause warnings at build time or runtime, so they're OK.

BTW I wasn't sure if I should base my branch on develop or on master, so I based it on master. If you want it to be on top of develop instead, just let me know and I can open a new PR with that basing.

stale[bot] commented 4 years ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

justingrant commented 4 years ago

Yes, still relevant. Waiting for a maintainer to review.

rkuykendall commented 4 years ago

~Hey @justingrant. Forked the project here, overhauled the build/publication process and actively accepting PRs:~

~https://github.com/rkuykendall/confetti-react~

stale[bot] commented 4 years ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

justingrant commented 4 years ago

Yes, still relevant. Waiting for a maintainer to review.

stale[bot] commented 4 years ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

justingrant commented 4 years ago

Yes, still relevant. Waiting for a maintainer to review.

stale[bot] commented 4 years ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

justingrant commented 4 years ago

Yes, still relevant. Waiting for a maintainer to review.

alampros commented 4 years ago

Thanks for this amazing PR, @justingrant ! (Seriously, kudos for the level of explanation)

Sorry for the long delay - I started a new job and got slammed.

justingrant commented 4 years ago

No worries, thanks @alampros for the kind words!

alampros commented 4 years ago

:tada: This PR is included in version 5.1.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

alampros / react-confetti

build: fix sourcemaps #79