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

module definition does not comply with ES6 modules and type definitions #78

Closed
RafaelSalguero opened this issue Dec 10, 2017 · 11 comments
Closed

Comments

@RafaelSalguero
Copy link

The way to import moize should be

import moize from "moize";

Instead one have to import it as

import * as moize from "moize"

Since the last two lines of the moize module are

exports.default = moize;
module.exports = exports['default'];

Instead of simply

exports.default = moize;

This does not comply with ES6 modules since a default import should return an object with a default property containing the default export value instead of returning directly the exported value

@planttheidea
Copy link
Owner

I'm not quite understanding the problem ... how does this surface? The way you describe how it should be imported is the way that is recommended, and the way you say you have to import it will not actually work since multiple exports to not exist... Each type (moize.simple, for example) is a property on the moize function, not a separate export.

Can you put together a codebin example that showcases the problem?

@RafaelSalguero
Copy link
Author

RafaelSalguero commented Dec 10, 2017

I'm writing the codebin, I think the problem lies on the babel-plugin-add-module-exports, since the correct way to export a default export is exports.default = mydefault and the plugin restores a babel 5 hack that was fixed on babel 6

Please check the thread babel/babel#2212

Another fix would be to change the typescript typings. at the end of the index.d.ts
replace

export default moize

with

export = moize

@planttheidea
Copy link
Owner

planttheidea commented Dec 10, 2017

Oh yes that creates the code you're talking about, I'm just not understanding the problem you describe with the result. Then implementing it in a variety of contexts, in has worked for me.

EDIT: Also, what you are describing is only true of the lib folder, which has the modules transpiled back to CommonJS. If you want to use the pure ES6 module syntax, use the export from the es folder ... this does not transpile the modules. This should be automatically triggered by build systems that support module declaration on package.json instead of main, but if not you can just do:

import moize from 'moize/es';

@RafaelSalguero
Copy link
Author

I made a simple repository showing the problem
https://github.com/RafaelSalguero/moizeDefaultExportProblem/tree/master

It looks like babel inserts the _interopRequireDefault as a fix for the old babel-5 export hack. Typescript does not implement this hack thus babel 5 modules are not compatible with typescript. This explanation is detailed on the microsoft/TypeScript#5565

@planttheidea
Copy link
Owner

Have you attempted what I recommended above? Importing instead from the es folder instead?

import moize from 'moize/es';

This does not have the same export override as that in the lib folder.

@RafaelSalguero
Copy link
Author

I made the test and node throws an error since import its not supported on node yet. This fails on the simple repository.

SyntaxError: Unexpected token import
    at createScript (vm.js:74:10)
    at Object.runInThisContext (vm.js:116:10)
    at Module._compile (module.js:537:28)
    at Object.Module._extensions..js (module.js:584:10)
    at Module.load (module.js:507:32)
    at tryModuleLoad (module.js:470:12)
    at Function.Module._load (module.js:462:3)
    at Module.require (module.js:517:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (C:\Users\rafael\Desktop\moize import\index.js:3:14)

@planttheidea
Copy link
Owner

planttheidea commented Dec 10, 2017

So just so that I understand the context that you are having this problem in:

  • You are using moize in a nodejs (non-browser) environment
  • You are using typescript (which requires the export default syntax instead of the module.exports syntax)

Is this correct? If so ... how can you consume any commonjs module? I don't use typescript, so I don't understand why it is assuming a default property would exist (because that seems like it would break any commonjs module that uses the module.exports syntax)?

EDIT: This issue has been discussed at length here: microsoft/TypeScript#2719

A possibility may be to provide an additional default property on the moize function that references itself:

moize.default = moize;

It feels janky, but it was one of the recommended courses of action and would solve this problem without breaking the current behavior. I'll experiment with this.

@planttheidea
Copy link
Owner

So I just created a PR (feel free to take a look), but like I said I don't use TS (the typings were provided by the community) so this is mainly educated guesswork. If this doesn't do the trick, I may need to lean on you or the community at large for a PR to provide a more proper resolution.

@RafaelSalguero
Copy link
Author

It look fine to me, I'll give it a test and summit another PR in case of any problem. Thanks!

@planttheidea
Copy link
Owner

Cool, I'll merge and release tonight once I get home.

@planttheidea
Copy link
Owner

Fix released as 4.0.4, let me know if you have any more issues.

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

No branches or pull requests

2 participants