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

Unnamed default exports don't work with --t es6 --m commonjs #5844

Closed
joeduffy opened this issue Dec 1, 2015 · 29 comments
Closed

Unnamed default exports don't work with --t es6 --m commonjs #5844

joeduffy opened this issue Dec 1, 2015 · 29 comments
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue High Priority

Comments

@joeduffy
Copy link

joeduffy commented Dec 1, 2015

Compiling the following code:

"use strict";
export default class {};

Using the command line tsc --t es6 --m commonjs produces illegal JavaScript:

"use strict";
class {
}
exports.default = default_1
;

I realize this is a weird combination of switches, however it seems necessary to target current versions of Node.js/V8 that don't yet natively support ES6 modules.

@weswigham weswigham self-assigned this Dec 1, 2015
@weswigham
Copy link
Member

This conditional needs to be updated to emit the temp name if the class is an exported default class when not targeting ES6. Forgot about unnamed classes when adding tests in #5648.

While looking at it, it would probably also be a good idea to verify all of the default export results with functions and unnamed function expressions as well...

@vvakame
Copy link
Contributor

vvakame commented Dec 1, 2015

same as unnamed function.

$ tsc -v
Version 1.8.0-dev.20151130
$ cat sub.ts
export default function (name: string) {
    return `Hi! ${name}`;
}
$ tsc --target es6 --module commonjs sub.ts
$ cat "use strict";
function (name) {
    return `Hi! ${name}`;
}
exports.default = default_1;

@mhegazy mhegazy added the Bug A bug in TypeScript label Dec 1, 2015
@mhegazy mhegazy added this to the TypeScript 1.8 milestone Dec 1, 2015
@nmalaguti
Copy link

Actually, I may have misunderstood the expected behavior for TypeScript 1.7 when targeting commonjs with ES6 modules.

This also appears to be a problem with named classes that are default exports.

$ tsc -v
message TS6029: Version 1.7.3

example.ts

// example.ts
export default class Example {
    public get() {
        return 1;
    }
}

subExample.ts

// subExample.ts
import Example from "./example";

export default class SubExample extends Example {
    public get() {
        return 3;
    }

    public foo() {
        return 2;
    }
}
$ tsc -t es6 -m commonjs example.ts subExample.ts

example.js

// example.js
class Example {
    get() {
        return 1;
    }
}
exports.Example = Example;

subExample.js

// subExample.js
var example_1 = require("./example");
class SubExample extends example_1.default {
    get() {
        return 3;
    }
    foo() {
        return 2;
    }
}
exports.SubExample = SubExample;

When run with node v5:

TypeError: Class extends value undefined is not a function or null
    at Object.<anonymous> (subExample.js:3:35)

@weswigham
Copy link
Member

That last bit duplicates #5594 and should be fixed in master. However your original issue still stands.

weswigham added a commit that referenced this issue Dec 1, 2015
Fix #5844 - add many new tests covering named/anonymous default exports
@mhegazy
Copy link
Contributor

mhegazy commented Dec 1, 2015

@joeduffy and @nmalaguti can you give typescript@next a try tomorrow and let us know if the issue has been resolved.

@joeduffy
Copy link
Author

joeduffy commented Dec 1, 2015

Thanks all, that was fast! I'll give it a try in the AM.

@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Dec 1, 2015
@nmalaguti
Copy link

This has resolved the issue with default named classes that I was seeing on 1.7 when using ts@next.

How does this interact with libraries compiled to commonjs using babel?

@joeduffy
Copy link
Author

joeduffy commented Dec 2, 2015

LGTM, works for me.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 2, 2015

How does this interact with libraries compiled to commonjs using babel?

not sure i understand the question.

@nmalaguti
Copy link

@mhegazy when babel emits code for es6 imports, it includes a method that looks for __esModule to determine whether to wrap the import in an object defining a default:

"use strict";

Object.defineProperty(exports, "__esModule", {
    value: true
});

var _example = require("./example");

var _example2 = _interopRequireDefault(_example);

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

class SubExample extends _example2.default {
    get() {
        return 3;
    }

    foo() {
        return 2;
    }
}
exports.default = SubExample;

If example.ts in my comment above is compiled with TypeScript, and subExample.ts is compiled with babel (simulating a babel project calling a TypeScript compiled project), then the compiled code for subExample.ts above breaks.

class SubExample extends _example2.default {
                                  ^

TypeError: Class extends value #<Object> is not a function or null
    at Object.<anonymous> (/Users/nmalaguti/git/ts-es6-commonjs/subExample.js:15:35)
    at Module._compile (module.js:425:26)
    at Object.Module._extensions..js (module.js:432:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:313:12)
    at Module.require (module.js:366:17)
    at require (module.js:385:17)
    at Object.<anonymous> (/Users/nmalaguti/git/ts-es6-commonjs/test.js:2:20)
    at Module._compile (module.js:425:26)
    at Object.Module._extensions..js (module.js:432:10)

The variable _example is { default: { default: [Function: Example] } } because __esModule was not set on it.

Is there a plan for how these will interop in the future?

@weswigham
Copy link
Member

Even if there isn't right now, as a workaround on your TS modules you can add the __esModule exported member yourself to force babel into doing the right thing with its interop require.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 3, 2015

@nmalaguti i think there is something else going on here. TypeScript compiler will emit the "__esModule", so compileing your example.ts results in:

// example.ts
var Example = (function () {
    function Example() {
    }
    Example.prototype.get = function () {
        return 1;
    };
    return Example;
})();
Object.defineProperty(exports, "__esModule", { value: true });
exports.default = Example;

@nmalaguti
Copy link

@mhegazy it looks like you targeted es5, not es6 (since the class was compiled into a function). The issue is when targeting es6 and using commonjs modules, the "__esModule" isn't emitted.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 3, 2015

I thought you were passing the es6 output into Babel...

@nmalaguti
Copy link

Sorry for the confusion. The use case would be I write an npm module in TypeScript with ES6 features like generators. I use tsc -t es6 -m commonjs to compile and publish.

A consumer of my library npm installs it and wants to use it with babel in order to take advantage of ES6 features. In that case, when they import my module as a default, babel will compile that down in a way that is incompatible since "__esModule" is missing in the TypeScript output.

@weswigham
Copy link
Member

Ah! So the issue is that we don't emit our __esModule shim when targeting ES6 with non-ES6 modules! I see!

@mhegazy
Copy link
Contributor

mhegazy commented Dec 3, 2015

i have a fix out for this one: #5907

@weswigham
Copy link
Member

Oh man, I just put out #5908 with almost the same change.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 3, 2015

great minds..

@mhegazy
Copy link
Contributor

mhegazy commented Dec 3, 2015

@nmalaguti give typescript@next a try later tonight and let us know if you are still running into issues.

@nmalaguti
Copy link

Thanks @mhegazy and @weswigham. I was able to get things working smoothly with typescript@next.

@jaumard
Copy link

jaumard commented May 27, 2016

Hum this was closed but I have the following problem :

My TS file :

'use strict'

import {Request, Response} from 'express'
import Controller from 'trails-controller'

/**
 * @module DefaultController
 *
 * @description Default Controller included with a new Trails app
 * @see {@link http://trailsjs.io/doc/api/controllers}
 * @this TrailsApp
 */
export default class DefaultController extends Controller {
  app: any;
  /**
   * Return some info about this application
   */
  info(req: Request, res: Response) {
    res.status(200).json(this.app.services.DefaultService.getApplicationInfo())
  }
}

The compiled result :

'use strict';
const trails_controller_1 = require('trails-controller');
class DefaultController extends trails_controller_1.default {
    info(req, res) {
        res.status(200).json(this.app.services.DefaultService.getApplicationInfo());
    }
}
Object.defineProperty(exports, "__esModule", { value: true });
exports.default = DefaultController;

How can I remove .default added in the extends and exports ? Because they not working in v8 I expect this output :

'use strict';
const trails_controller_1 = require('trails-controller');
class DefaultController extends trails_controller_1 {
    info(req, res) {
        res.status(200).json(this.app.services.DefaultService.getApplicationInfo());
    }
}
module.exports = DefaultController;

@mhegazy
Copy link
Contributor

mhegazy commented May 31, 2016

I wil have to look at the trails-controler definition to be able to help. You are importing it as a default export, so the output is what I would expect.

@jaumard
Copy link

jaumard commented May 31, 2016

@mhegazy thanks. trails-controller is a node third module, a simple class :

'use strict'

/**
 * Trails Controller Class.
 */
module.exports = class TrailsController {

  constructor (app) {
    Object.defineProperty(this, 'app', {
      enumerable: false,
      value: app
    })
  }

  /**
   * Controller configuration
   */
  static config () {
  }

  /**
   * Return the id of this controller
   */
  get id () {
    return this.constructor.name.replace(/(\w+)Controller/, '$1').toLowerCase()
  }

  get log () {
    return this.app.log
  }

  get __ () {
    return this.app.__
  }
}

If this module doesn't export his class the right way, what is the right way to export it ? I'll make a PR on it.
https://github.com/trailsjs/trails-controller

@mhegazy
Copy link
Contributor

mhegazy commented May 31, 2016

your import should look like import Controller = require("trails-controller"); instead.

the reason is import Controller from 'trails-controller' imports the default export of the module, and this module is a pre-ES6 module, i.e. it does not have a default export.

@jaumard
Copy link

jaumard commented May 31, 2016

@mhegazy thanks ! That's working (with "noLib": true,) because if I remove this flag it doesn't know about trails-controller. I try to create a .d.ts file but can't manage how to declare the class correctly :(
I try :

declare module "trails-controller" {
  export default class Controller {
    app: any
  }
}

With or without default I get ViewController.ts(27,47): error TS2507: Type 'typeof "trails-controller"' is not a constructor function type.

@mhegazy
Copy link
Contributor

mhegazy commented May 31, 2016

class Controler {...}
export=Controler;

@scott-wyatt
Copy link

I'm trying to write a definition for the trails-controller that @jaumard mentioned, but I lack the experience. Is this something close?

declare module TrailsController {
  export class TrailsController {
    app: {
      config: any;
      sitemap: any;
      routes: { path: string, config: {app?: any} }[];
      services: any;
    };
    log: any;
    id: any;
    __: any;
    config: any;

    constructor(app: any);
  }
}
declare module "trails-controller" {
  export = TrailsController;
}

@kitsonk
Copy link
Contributor

kitsonk commented Oct 4, 2016

General questions should be on StackOverflow, Gitter or IRC, but the following works:

declare namespace TrailsController {
  class TrailsController {
    app: {
      config: any;
      sitemap: any;
      routes: { path: string, config: {app?: any} }[];
      services: any;
    };
    log: any;
    id: any;
    __: any;
    config: any;

    constructor(app: any);
  }
}

declare module "trails-controller" {
  const TrailsController: TrailsController.TrailsController;
  export = TrailsController;
}

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue High Priority
Projects
None yet
Development

No branches or pull requests

8 participants