Skip to content

chore(sourcemaps): simplify transformation of sourcemaps#3026

Merged
rwaskiewicz merged 1 commit intorwaskiewicz-rebase-again-sourcemapsfrom
rwaskiewicz/sourcemap-transform
Aug 25, 2021
Merged

chore(sourcemaps): simplify transformation of sourcemaps#3026
rwaskiewicz merged 1 commit intorwaskiewicz-rebase-again-sourcemapsfrom
rwaskiewicz/sourcemap-transform

Conversation

@rwaskiewicz
Copy link
Copy Markdown
Member

@rwaskiewicz rwaskiewicz commented Aug 25, 2021

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally and any changes were pushed
  • Tests (npm run test.karma.prod) was run locally and passed

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

simplify the transformation of sourcemaps by removing code paths that I
don't believe will be followed.

GitHub Issue Number: N/A

What is the new behavior?

do a 1:1 mapping of the rollup representation of a sourcemap to a Stencil one. I did briefly consider removing this altogether, but I think that there's value in having our own representation/abstraction to mitigate any damage that could be caused by the typings of a sourcemap in rollup occurring

Does this introduce a breaking change?

  • Yes
  • No

Testing

After checking this commit out, I:

  • installed dependencies (npm ci)
  • built the project and generated the tarball for it (npm run build && npm pack)
  • installed it on a fresh stencil project (npm stencil init && npm i <PATH_TO_TARBALL>)
  • started the project (npm start)
  • once Stencil's dev server had opened chrome, I opened dev tools and verified sourcemaps were still generated

Other information

N/A

simplify the transformation of sourcemaps by removing code paths that I
don't believe will be followed.

add tests to the sourcemaps utility function
@rwaskiewicz rwaskiewicz marked this pull request as ready for review August 25, 2021 20:01
@rwaskiewicz rwaskiewicz requested a review from a team August 25, 2021 20:01
Copy link
Copy Markdown
Contributor

@splitinfinities splitinfinities left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to understand the purpose behind the return signature a little better, but I don't have any blocking changes. I appreciate the readability and how much more straight-forward the resultant object became.

@rwaskiewicz
Copy link
Copy Markdown
Member Author

@splitinfinities LMK if I answered your question sufficiently in the comments, happy to elaborate if not

@splitinfinities
Copy link
Copy Markdown
Contributor

@rwaskiewicz it all makes sense to me now 👍

@rwaskiewicz rwaskiewicz merged commit 4c46f7c into rwaskiewicz-rebase-again-sourcemaps Aug 25, 2021
rwaskiewicz added a commit that referenced this pull request Aug 27, 2021
simplify the transformation of sourcemaps by removing code paths that I
don't believe will be followed.

add tests to the sourcemaps utility function
@rwaskiewicz rwaskiewicz deleted the rwaskiewicz/sourcemap-transform branch September 9, 2021 21:31
rwaskiewicz added a commit that referenced this pull request Sep 20, 2021
simplify the transformation of sourcemaps by removing code paths that I
don't believe will be followed.

add tests to the sourcemaps utility function
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.

2 participants