Closed Aristona closed 5 years ago
I'm unable to replicate. Could this be an issue in your environment?
Sorry for late response.
I have the same issue.
This mismatch causes the problem.
What's going on is that rollup changes your default export to a namespace export.
See the documentation on output.exports here: https://rollupjs.org/guide/en#advanced-functionality
So it seems you should either change the typescript definition to match what rollup produces or change the rollup configuration to match the typescript definition.
Alternatively, include a third version generated using format "esm" instead of "cjs" and point the package.json module field to that.
Ok, I looked into it more, but am still unable to replicate. Keep in mind I'm a typescript novice.
I set up a Typescript project based on this starter. As you can see here I imported the Switch as normal, and it compiled completely fine in both dev and production mode. I also got correct typescript errors when I tried to add the props with wrong types to the Switch-component. Why do you think it works here but not for you guys? Could someone link a repo with replication of the issue?
As far as exporting goes: Currently the compiled files in the dist directory are exported like this: module.exports = ReactSwitch
. Then the index.js file exports either the prod- or dev-bundle in the same way. Isn't this a correct way to export something as default using commonjs? I tried to change the rollup configs ´output.exports´ to default
, but it didn't change the output.
Ok.
You can reproduce the issue by removing the
"module": "esnext",
Line from the tsconfig.json file.
If you look at a transpiled ESModule, it looks something like this:
Object.defineProperty(exports, "__esModule", {
value: true
});
exports.default = ReactSwitch;
You'll notice that it doesn't store the ReactSwitch on exports but on exports.default.
If you use babel and import ReactSwitch, you get something like this:
var _reactSwitch = require('react-switch');
var _reactSwitch2 = _interopRequireDefault(_reactSwitch);
function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }
You'll see that it checks for the __esModule
property and treats it differently depending on the value. This allows you to import both transpiled esmodules and commonjs modules transparently in babel.
In contrast, if you compile via Typescript, you'll get something like:
var react_switch_1 = require("react-switch");
react_switch_1["default"];
TypeScript doesn't (in its default configuration) treat commonjs and transpiled es modules transparently. Instead if you take the default import, it will assume its a transpiled es module and try to grab the default property.
In order to get the whole module.exports in typescript, you instead need to use the syntax
import * as ReactSwitch from 'react-switch
However, if you use "module": "esnext", as in your sample, the typescript compiler doesn't rewrite the imports as require
calls, but instead puts the imports in the output. Then the imports get resolved by something else. In the case of your sample, I believe it ends up being webpack which supports the same logic as babel, which is why it works. With module: esnext, typescript doesn't handle the module resolution.
You can get rollup to generate an transpiled esmodule instead of a commonjs module by using the settings: esModule: true
and exports: named
. However, that will break anyone who is using the module via require. In that case the actually ReactSwitch as would be at require('react-switch')["default"]
.
You can change your typescript file to export ReactSwitch as a namespace, which will require users to use import * as ReactSwitch from 'react-switch'
. But that will break anyone who is currently using the module as you do in the sample.
You could tell everyone that they need to use module: esnext or esModuleInterop: true in their typescript config. Having module: esnext is probably almost always a good idea anyways. But you may not want to require specific typescript settings to use your module.
You could also include an es version of your module. It would contain export and imports instead of requires. I think this would work in all cases. It would also allow concating your module with the other modules, whereas as it stands it has to fallback onto simulating the require framework.
`
Thanks for the in-depth explanation. You seem to know your stuff.
I'm having a busy week at work so I'll see if I can fix it this weekend. The goal is of course to support the standard typescript config.
I'll take a look at the options you mentioned and evaluate. Namespace export is probably off the table since it sounds like that would break all existing uses? (even though easy for people to fix). ESM probably isn't happening either. I'm kind of commited to commonjs at this point, since es modules as far as I know doesn't allow webpack to serve different dev and prod-bundles. It would make the final bundle size footprint slightly larger.
I'm gonna take a look at how the React repo does it as well since they also serve commonjs with separate dev and prod-bundles. Clearly they don't have the same problem.
I can attest that React does an export namespace, you have to "import * as React" in order to use it. If you look inside your sample repo, you'll find that's the form used to import React.
Actually, I think using an EM module might well make the bundle size footprint slightly smaller. The deal is that ES modules which can be concatenated with all my other ES modules have less overhead bundle-wise than CJS modules. The only thing stopping that is your propTypes assignment. I think that if you wrap that in a test for development mode the treeshaking on the bundler should get rid just as much of that as your current configuration does.
On Tue, Jan 8, 2019 at 1:32 PM Markus Englund notifications@github.com wrote:
Thanks for the in-depth explanation. You seem to know your stuff.
I'm having a busy week at work so I'll see if I can fix it this weekend. The goal is of course to support the standard typescript config.
I'll take a look at the options you mentioned and evaluate. Namespace export is probably off the table since it sounds like that would break all existing uses? (even though easy for people to fix). ESM probably isn't happening either. I'm kind of commited to commonjs at this point, since es modules as far as I know doesn't allow webpack to serve different dev and prod-bundles. It would make the final bundle size footprint slightly larger.
I'm gonna take a look at how the React repo does it as well since they also serve commonjs with separate dev and prod-bundles. Clearly they don't have the same problem.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/markusenglund/react-switch/issues/20#issuecomment-452459303, or mute the thread https://github.com/notifications/unsubscribe-auth/AA0ZfUizDzZejQCxstfD_FKogiUHXfu0ks5vBQ6DgaJpZM4X58BL .
Looks like exports: named
is probably the way to go, although I'm not certain that it's worth breaking people using commonjs syntax in favor of making it work for typescript users who don't have the esModuleInterop
flag set to true. I was hoping to release the new version this weekend but it got delayed by a weird bug the new version has in IE 11 :(
I ended up not shipping exports:named
in v4.0, due to fear of breaking something...
I just ran into this issue and it was very confusing because of how obscure the error message is.
I would strongly recommend switching to the "export namespace" form that React uses. Semantic versioning is suppose to allow you to make breaking changes, so I would recommend doing this for v5.0.
At least add documentation to the README to add esModuleInterop: true
to the tsconfig.json.
I took your advice and made the change for 5.0.0 which was just released. This should no longer be a problem.
Hi,
I am having the following error when I try to use this library.
The following code:
produces:
Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports. Check the render method of
Component.
It works when I use * import syntax, but TypeScript gives the following error in that case.
`JSX element type 'Switch' does not have any construct or call signatures.