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

@rspack/plugin-html only providing simulated default export #2851

Closed
shawnmcknight opened this issue Apr 20, 2023 · 4 comments · Fixed by #3358
Closed

@rspack/plugin-html only providing simulated default export #2851

shawnmcknight opened this issue Apr 20, 2023 · 4 comments · Fixed by #3358
Labels
breaking change feat New feature or request stale

Comments

@shawnmcknight
Copy link

System Info

System:
OS: Windows 10 10.0.19045
CPU: (16) x64 11th Gen Intel(R) Core(TM) i9-11900H @ 2.50GHz
Memory: 17.15 GB / 31.73 GB
Binaries:
Node: 18.15.0 - C:\Program Files\nodejs\node.EXE
npm: 9.5.0 - C:\Program Files\nodejs\npm.CMD
Browsers:
Edge: Spartan (44.19041.1266.0), Chromium (112.0.1722.48)
Internet Explorer: 11.0.19041.1566

Details

Originally reported in #2795 but splitting into separate issue.

@rspack/plugin-html ships with a cjs build but that cjs build only provides a simulated default export. That is, the exports object has a default property on it, but exports itself is not assigned to anything:

exports.default = HtmlRspackPlugin;

Using the are the types wrong tool, it will report:

The CJS module resolved at the package under contains a simulated export default with an __esModule marker, but no top-level module.exports. Node does not respect the __esModule marker, so accessing the intended default export will require a .default property access in Node from an ES module.

Effectively, this doesn't allow for an expected esm import or cjs require. To use cjs one must:

const { default: HtmlRspackPlugin } = require('@rspack/plugin-html');

and to use esm one must:

import HtmlRspackPlugin from '@rspack/plugin-html';
...
	const plugins = [
		new HtmlRspackPlugin.default({
...

I'm guessing this syntax requirement is unintentional.

Reproduce link

No response

Reproduce Steps

Try using @rspack/plugin-import with either:

const HtmlRspackPlugin = require('@rspack/plugin-html');

or

import HtmlRspackPlugin from '@rspack/plugin-html');

If you try to new HtmlRspackPlugin() you will get an error about it not being constructable.

@shawnmcknight shawnmcknight added bug Something isn't working pending triage The issue/PR is currently untouched. labels Apr 20, 2023
@hyf0 hyf0 added feat New feature or request p3-nice-to-have and removed bug Something isn't working pending triage The issue/PR is currently untouched. labels Apr 21, 2023
@hyf0
Copy link
Collaborator

hyf0 commented Apr 21, 2023

@ahabhgk cc

@hyf0 hyf0 added the to be discussed Rspack team would discuss these issues per week label Apr 21, 2023
@ahabhgk
Copy link
Collaborator

ahabhgk commented Apr 21, 2023

@shawnmcknight Do you want to create a PR for this?

@shawnmcknight
Copy link
Author

@shawnmcknight Do you want to create a PR for this?

I can, but I'm not sure there is a way to get the tsc compiler to support a cjs "default" export (i.e. module.exports=HtmlRspackPlugin as well as allowing for cjs style "named" exports (i.e. module.exports.foo=Foo).

Currently the lib is exporting the HtmlRspackPlugin as an esm default export as well as exporting multiple named interfaces as well as defaultTemplateCompiler. The easiest path forward would be to just export HtmlRspackPlugin as a named export but that would make the implementation different than the equivalent webpack plugin.

Alternatively, I believe I can make everything work with module.exports=HtmlRspackPlugin if the defaultTemplateCompiler doesn't need to be exported. I'm not sure if there's a reason that's exported today.

I believe those are the only two viable approaches due to limitations in tsc's compilation output. Any thoughts on which is preferable?

@hardfist
Copy link
Contributor

I think use named exports is the right choice but it's definitely a breaking change, we will fix this when release 0.2.0 version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change feat New feature or request stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants