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

fix: @parcel/source-map default import #157

Merged

Conversation

bashmish
Copy link
Contributor

Fixes #155

@changeset-bot
Copy link

changeset-bot bot commented Aug 31, 2023

🦋 Changeset detected

Latest commit: 98f022c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@chialab/estransform Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

js: `import { createRequire as __moduleCreateRequire } from 'module';
const require = __moduleCreateRequire(import.meta.url);
`,
},
Copy link
Contributor Author

@bashmish bashmish Aug 31, 2023

Choose a reason for hiding this comment

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

I used npm run build as a way to test this PR and this is how I found out that this require is needed in the new output

any other automated tests I can run? I didn't understand it from package.json or docs, npm test doesn't seem to do the job

I tested in my repository and the new dist works

@bashmish
Copy link
Contributor Author

bashmish commented Sep 1, 2023

@edoardocavazza I'd really appreciate if you can review this :)

Copy link
Member

@edoardocavazza edoardocavazza left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! I have proposed a change that should work the same way, but with (IMO) less complexity.

packages/estransform/lib/sourcemaps.js Outdated Show resolved Hide resolved
@bashmish bashmish force-pushed the fix/parcel-source-map-default-import branch from 3337905 to 98f022c Compare September 4, 2023 09:06
@bashmish
Copy link
Contributor Author

bashmish commented Sep 4, 2023

@edoardocavazza fixed as per your suggestion using createRequire, thanks!

@codecov
Copy link

codecov bot commented Sep 4, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (5f0f3c9) 90.99% compared to head (98f022c) 90.99%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #157   +/-   ##
=======================================
  Coverage   90.99%   90.99%           
=======================================
  Files          39       39           
  Lines        5928     5929    +1     
=======================================
+ Hits         5394     5395    +1     
  Misses        534      534           
Files Changed Coverage Δ
packages/estransform/lib/sourcemaps.js 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@edoardocavazza edoardocavazza merged commit 71a9040 into chialab:main Sep 4, 2023
8 checks passed
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.

SourceMapNode is not a constructor
2 participants