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

Avoid having to use esModuleInterop #43

Closed
danikaze opened this issue Mar 22, 2022 · 4 comments · Fixed by #44 or #57
Closed

Avoid having to use esModuleInterop #43

danikaze opened this issue Mar 22, 2022 · 4 comments · Fixed by #44 or #57

Comments

@danikaze
Copy link
Contributor

ES6 modules are supposed to be an object, however this package exports a function by default.
To make it work in TypeScript it requires this option set to true, which means extra code in the build... and would be nice not requiring it.

now:

import clsx from 'clsx';

after:

import { clsx } from 'clsx';

Since this would be a breaking change, I would suggest applying this fix in a way that the old version is also supported -which requires a bit of dirty code- and then apply the clean version in a major version up release :)

// the dirty hack would look like this (types also required)
module.exports.clsx = module.exports;

// so you actually can do both in this version preserving compatibility
import clsx from 'clsx';
import { clsx } from 'clsx';

next version (let's say a major version up) could just export the function like this:

function clsx () {
	var i=0, tmp, x, str='';
	while (i < arguments.length) {
		if (tmp = arguments[i++]) {
			if (x = toVal(tmp)) {
				str && (str += ' ');
				str += x
			}
		}
	}
	return str;
}

module.exports = { clsx };

With this migration we are achieving two things:

  1. Following ES6 standards
  2. Requiring less code when compiling in typescript since esModuleInterop wouldn't be required anymore
@lukeed
Copy link
Owner

lukeed commented Mar 22, 2022

This module mirrors classnames for drop-in usage.

You should be using a bundler, not tsc directly for application outputs. All bundlers can find & resolve clsx as ad default export just fine w/o use of an interop layer. This is because the "module" field exists and is respected by all of them.

@danikaze
Copy link
Contributor Author

And with this change it will still acting the same, with the difference that it will comply with ES6 modules as well.

I tried it in a project without typescript esModuleInterop option and this is the result:

TypeError: (0 , clsx_1.default) is not a function

So yes, a bundler (webpack) is used, but in this project (big company one, hard to change configs) is not working properly.
Anyways, consider the open PR since it's a small change and 100% backwards-compatible :)

@scott-lc
Copy link

Can't use clsx with the upcoming TypeScript 4.7 nodenext resolution mode. PR #44 would fix this.

microsoft/TypeScript#49189

@CanRau
Copy link

CanRau commented Jun 30, 2022

One workaround might be import {default as clsx} from "clsx", not sure that works in all cases though

@lukeed lukeed closed this as completed in #44 Jul 2, 2022
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.

4 participants