-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
ESM build does not export named exports #3295
Comments
How so specifically?
This is not required (AFAICT) and the instructions go on to state that explicitly:
HLJS is designed to provide a singleton instance, so we've always exported an instance (as default)... it makes sense to continue to export this main instance in ESM just as it did in CJS.
It's right in the comments, though I guess the link was subtly off: https://nodejs.org/api/packages.html#packages_dual_package_hazard This forces a SINGLE instance of HLJS, even if both ESM and CJS are libs imported, this is why a wrapper. |
I don't know what this means exactly... does your project fail to build? What is the error? Does it simply fail at runtime, is there a console error, etc? It's worth noting the CDN build now also includes ESM libs, I wonder if that might perhaps be of use to you. It is done differently there since we don't have to take Node idiosyncrasies into account. |
The error I'm getting is:
The problem is that SvelteKit runs as an isomorphic web app. It expects the same code to be able to be used on both the client and server. That's not possible when you have to use default exports because default exports need to be referenced differently depending on whether the package is CJS or ESM. It would also be good practice to define the |
So use us as ESM everywhere? Not sure I follow the problem.
We already have an exports map. We purposely chose to stay CJS for
Sadly, to me, that sounds like a limitation your bundling software perhaps needs to address. (if it's causing real pain with integration) We've provided only a default export for years with no issues at all... when we started publishing our ESM builds and people starting using them also, no issues (with our default export). I'm not opposed to ALSO adding a |
If we did do this we'd still only have two exports:
We wouldn't be exporting individual functions. |
Are you using our |
Are you talking about something like this? #3307 |
Yes, #3307 should work if it also added a named export to the CJS version as well. I'm using the |
How is that done? module.exports = hljs;
module.exports.hljs = hljs; ? |
Yeah. I think that'd work. Or:
|
@benmccann Could you build and test PR #3307 and see if it works for you? |
@benmccann Ping. |
Yes, it does work. Thank you! |
I'm curious though now that we're adding |
We've actually gotten |
So technically from your point of view this is no longer necessary? If there is a chance default will work I'd prefer just to have the one rather than the both. |
Yeah, we shouldn't need this anymore even if it'd be good practice to use named exports instead of default exports |
Published with 11.3. The export is now (also) named as |
Thanks! |
Also not working with rollup. |
Describe the issue/behavior that seems buggy
The ESM build does not export named exports.
The highlight code links to the Node docs on how to setup an ESM build:
highlight.js/tools/build_node.js
Line 11 in bd548da
However, it does not appear to follow the instructions mentioned there.
In my experience, the wrapper approach mentioned in the Node docs doesn't work all that well compared to having Rollup output both formats. I see you're already using Rollup. Is there any reason you don't have it output CJS and ESM instead of creating a wrapper?
Sample Code or Instructions to Reproduce
Expected behavior
The individual functions like
highlight
andregisterLanguage
should be exported in addition to the default exportAdditional context
I can't get highlight to work in SvelteKit / Vite without workarounds (metonym/svelte-highlight#158)
The text was updated successfully, but these errors were encountered: