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

Include cjs in release #1686

Merged
merged 2 commits into from
Jan 31, 2023
Merged

Include cjs in release #1686

merged 2 commits into from
Jan 31, 2023

Conversation

benjaminleonard
Copy link
Contributor

@benjaminleonard benjaminleonard commented Jan 30, 2023

Hi, when importing @asciidoctor/core@3.0.0-alpha.4 with Remix (https://remix.run/) I get the following error:

TypeError: The "path" argument must be of type string or an instance of URL. Received undefined

Applying to this line:

const __asciidoctorDistDir__ = path.dirname(fileURLToPath(import.meta.url))

This seems to be because import.meta.url is not available in cjs. Essentially, Remix converts esm packages to cjs to run on the server (see here: https://remix.run/docs/en/v1/pages/gotchas#importing-esm-packages).

I'm proposing we keep the release as it is, but copy over the cjs as a fallback so it can be used as a dual CommonJS/ES module package.

@mojavelinux
Copy link
Member

I'm in favor as Antora will need this as well. I have no plans to switch Antora to ESM right now as it requires a major change to the build process. Having a CJS export would be very helpful.

@ggrossetie ggrossetie merged commit e8b3456 into asciidoctor:main Jan 31, 2023
@ggrossetie
Copy link
Member

Thanks!

@benjaminleonard
Copy link
Contributor Author

If you get the chance I would love an alpha release including this PR.

Thanks!

@ggrossetie
Copy link
Member

At this point, I think I should release a "rc".
I've been experimenting a bit with Asciidoctor.js 3.x ESM and I haven't noticed any issue.

I will probably upgrade to the latest release of Opal as well (from 1.5.1 to 1.7.3).

@benjaminleonard
Copy link
Contributor Author

That'd be perfect! Thanks

@ggrossetie
Copy link
Member

@benjaminleonard I just released 3.0.0-rc.1 but I noticed that Vite ignores the browser field. I'm now using the following exports: ef422ba

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.

None yet

3 participants