Skip to content
This repository has been archived by the owner on Oct 9, 2020. It is now read-only.

exports is not defined (jspm bundles, Angular html template import in typescript - commonjs) #47

Open
MoonStorm opened this issue Mar 29, 2017 · 15 comments

Comments

@MoonStorm
Copy link

When using this plugin to import html files with an import statement in typescript, set to produce commonjs modules, I'm getting exports is not defined, in the browser, from the bundles created via jspm. The output looks something similar to:

System.register("app/components/component.html", [], function (_export, _context) {
  "use strict";
  return {
    setters: [],
    execute: function () {
      Object.defineProperty(exports, "__esModule", { value: true });
      exports.__useDefault = true;
      exports.default = "<div>....</div>";
    }
  };
});

No problem when transpiling directly in the browser though.

I changed the plugin to

'use strict';
module.exports = {
    translate: function (load) {
        load.metadata.format = 'cjs';
        var output =
        'module.exports = function () {' +
        '       return module.exports["default"].apply(this, arguments);' +
        '};'+
        'Object.defineProperty(module.exports, "default", {value: '+JSON.stringify(load.source)+'});';

        return output;
    }
}

and nothing complains any more. The generated js through the bundle looks like:

System.registerDynamic("app/components/component.html", [], true, function ($__require, exports, module) {
  var global = this || self,
      GLOBAL = global;
  module.exports = function () {
    return module.exports["default"].apply(this, arguments);
  };
Object.defineProperty(module.exports, "default", { value: "<div>...</div>" });
});

Also notice the difference between using registerDynamic vs register.
Any idea what's causing this?

@MoonStorm MoonStorm changed the title exports is not defined (Angular html template import in typescript - commonjs) exports is not defined (jspm bundles, Angular html template import in typescript - commonjs) Mar 29, 2017
@guybedford
Copy link
Member

Thanks for posting... this seems to be specifically related to the TypeScript plugin itself, where SystemJS builder thinks the format of the output is still an ES module and transpiles it, while the TypeScript plugin has already transpiled into CommonJS.

I'd suggest trying posting an issue on the TypeScript plugin project, and feel free to copy me in there.

@MoonStorm
Copy link
Author

MoonStorm commented Mar 30, 2017

Looking at the source code, the text plugin is the one setting the format to ES. The Typescript plugin isn't touching the file. When I change the text plugin to set the format to CJS, everything is working fine.

@guybedford
Copy link
Member

If the TypeScript plugin is configured as the transpiler, then I think the ES module will go through TypeScript first, before going through Rollup.

@aluanhaddad
Copy link

@guybedford

this seems to be specifically related to the TypeScript plugin itself, where SystemJS builder thinks the format of the output is still an ES module and transpiles it, while the TypeScript plugin has already transpiled into CommonJS.

Indeed, I spent some time trying reproduce this yesterday. One issue seems to be that plugin-typescript overwrites the format metadata when it transpiles modules based on the module format specified as the target module format used in typeScriptOptions. I played around with the various TypeScript module targets crossed with various metadata.format values and got everything from the issue reported here to nested System.register calls within a single output of the text plugin.

It's also worth noting that it's not simply the text plug-in that is affected difference in behavior between running bundled vs. unbundled. If you tell TypeScript to output es2015 modules, enabling Rollup optimizations, transpiling in the browser fails immediately, but bundling works perfectly.

My understanding is that SystemJS invokes the transpiler in a distinct pass to transpile ESModule import and export and that metadata.loader performs the compilation on the module body in a separate pass.

As you say this seems to be a plugin-typescript specific issue but I wanted to make note of it here.

@MoonStorm do you plan to open an issue with plugin-typescript? Feel free to copy me on it as well.

@frankwallis
Copy link

The problem is that when outputting commonjs typescript will always transpile

import template from './template.html'
console.log(template);

to

var template_html_1 = require("./template.html");
console.log(template_html_1.default);

but plugin-text outputs the string as an amd default export when running the browser and as an esm default export when bundling. This makes interoperability rather difficult.

I would suggest changing the amd mode to output the string as the 'default' property and setting the esModule flag, ie.

  load.metadata.format = 'amd';
  load.metadata.esModule = true;
  return 'def' + 'ine(function() {\nreturn { default: ' + JSON.stringify(load.source) + ' };\n});';

This will mean that the in browser behaviour and bundler behaviour are equivalent.

@frankwallis
Copy link

Unfortunately the above solution breaks when using vanilla commonjs :(. I don't see an easy solution to this.

@MoonStorm can I ask why you have typescript configured to output commonjs instead of system format?

@aluanhaddad
Copy link

@frankwallis all of Angular 2's in browser demos, plunkrs, and whatnot, have been using this setting for over a year, resulting in endless warnings. I have no idea why as register is clearly preferable.

@MoonStorm
Copy link
Author

MoonStorm commented Apr 2, 2017

@frankwallis I have attemped twice to switch to system module format. There is always some dependency that's not playing nice. I've wasted a lot of time trying to make this work.

@guybedford
Copy link
Member

@frankwallis @aluanhaddad thanks for the updates, yeah TypeScript not supporting the __esModule interop pattern we invented for these use cases seems to be the underlying issue (and that the text plugin is altering its behaviour between dev and production!).

I guess we should really be evangelising the system output from TypeScript then much more?

@aluanhaddad
Copy link

@guybedford as of 2.2 TypeScript does add the marker when transpliling to CommonJS but the problem is that Angular hasn't upgraded yet so I don't think their CommonJS distributions have it.

More broadly, I don't think TypeScript adds it when transpliling to AMD or UMD.

I can open an issue on the TypeScript repo for that.

@aluanhaddad
Copy link

@MoonStorm which libraries are you having trouble with? let me know and I'll be happy to give you a hand and see if we can add whatever changes may be appropriate to the jspm registry.

@guybedford
Copy link
Member

@aluanhaddad nice, that's good to know.

@MoonStorm
Copy link
Author

MoonStorm commented Apr 7, 2017

@aluanhaddad Thanks for offering to help but I'm not going to switch any time soon as the waters are still way too muddy. After spending days applying fixes left and right I remember that the last drop was when expanding observables in rxjs with custom operators wasn't working any more.

I'll be using my custom cjs loader for the time being. Works like a charm.

@aluanhaddad
Copy link

@MoonStorm glad you have a solution.

@guybedford, @frankwallis if System.register is the only supported format then we should clarify that as there are millions of plunkrs that use CommonJS with TypeScript and SystemJS.

Something interesting to note is that in TypeScript 2.3 there will be a custom transformations API. I haven't looked into it in detail yet but it might allow plugin-typescript to work better as a transpiler plugin (vs as a loader) in workflows that use "module": "es2015", perhaps allowing it to behave more like Babel.

@guybedford
Copy link
Member

@aluanhaddad it could be worth getting the main examples of the tool changed to the system pattern if that is preferable. But if the next TypeScript version will add the __esModule flag then it won't strictly necessary, as then semantically I don't think the benefit of System.register would be absolutely necessary for these use cases.

Good to know about the transformations API as well!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants