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

SourceMapNode is not a constructor #155

Closed
bashmish opened this issue Jul 3, 2023 · 3 comments · Fixed by #157
Closed

SourceMapNode is not a constructor #155

bashmish opened this issue Jul 3, 2023 · 3 comments · Fixed by #157

Comments

@bashmish
Copy link
Contributor

bashmish commented Jul 3, 2023

This is similar to #150

You have the following code in estransform:

// packages/estransform/lib/sourcemaps.js
import sourceMapDefault from '@parcel/source-map';

const { default: SourceMapNode } = sourceMapDefault;

which breaks when used in Node, because default export is already an object SourceMapNode, so extra destructuring makes it undefined leading to

TypeError: SourceMapNode is not a constructor

If I understood correctly from you previous answer, you have a browser ESM usage mainly, not Node like in my case.
Probably you have some compatibility mode for default exports which makes it work like this.

The original package has only regular default export https://cdn.jsdelivr.net/npm/@parcel/source-map@2.1.1/index.d.ts, so I think it's worth supporting such usage.

@edoardocavazza
Copy link
Member

Hello @bashmish, thank you for the issue and sorry for the delay.

I'm not sure if I understood the problem correctly. Indeed, the typings of @parcel/source-map suggest a default export of SourceMap, but this is not true on the implementation side.

As you can see here: link, the module exports the class like this:

exports.default = NodeSourceMap;

But it should be:

exports = NodeSourceMap;

in order for the class to be imported as the default member.

As you mentioned, we prefer Node with ESM, so I'm wondering if there's some scenario where the import is modified by a transpiler. Could you provide a reproduction case for us to investigate?

Nevertheless, we are certainly available to handle the scenario as soon as we understand it :)

@bashmish
Copy link
Contributor Author

@edoardocavazza I ran into similar issues just yesterday, so good timing, since it gave me more knowledge to answer :)

Indeed there is a transpiler somewhere which dynamically patches ESM modules to be consumed by CommonJS modules. I don't know exactly how to solve it yet, I might still require a fix on your side, but for the time being I'll close this issue. Thank you so far!

@bashmish
Copy link
Contributor Author

bashmish commented Aug 31, 2023

I'd like to reopen this.

Did a bit more digging into this problem and found out it's the well-known issue with default exports CJS/ESM interop which can't be solved by tools automatically in all cases.

There is a good explainer here https://esbuild.github.io/content-types/#default-interop

In my case I end up having The Babel interpretation somewhere in my toolchain. It's hard to pin-point where exactly as I have multiple layers of tools, which I don't always control, but I tried to play with configs after learning the heuristics that esbuild uses when choosing either The Babel interpretation or The Node interpretation, yet I couldn't make it work with current estransform published code.

In your case the The Node interpretation takes place and that requires you use this code:

import sourceMapDefault from '@parcel/source-map';
const { default: SourceMapNode } = sourceMapDefault;

This is a good workaround for apps, but for libs I think there is a better one which can solve it both for you and me. On top of that the TypeScript required you to copy over some types into packages/estransform/modules.d.ts from @parcel/source-map which can also be improved.

This problem and workarounds for it are also discussed in this esbuild issue.

@bashmish bashmish reopened this Aug 31, 2023
@bashmish bashmish changed the title SourceMapNode is not a constructor when used in Node SourceMapNode is not a constructor Aug 31, 2023
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 a pull request may close this issue.

2 participants