ShMcK / Angular2-Meteor-Demos

Angular2-Meteor Demos
14 stars 3 forks source link

Possible bug with imports #1

Open sclausen opened 9 years ago

sclausen commented 9 years ago

Maybe it's a selfmade issue, but I forked netanelgilad:angular2-typescript to update typescript to 1.5.3 (removes the require error in frontend). After I upgraded to 1.5.3 I noticed, all *.ts files are compiled with angular's modules so I changed netanelgilad's plugin to just transpile *.ng.ts files with angular2 modules and made a sclausen:angular2-typescript. After this I noticed in ng2-socially/04-forms-events/client/party-form/party-form.ts the line

import {formDirectives, Control, ControlGroup, Validators} from 'angular2/angular2';

doesn't work anymore, since the modules have to be imported from 'angular2/forms' not 'angular2/angular2'. Now I'm wondering why the import worked with netanelgilad:angular2-typescript in the first place. I think the error which occurred after I used my forked and modified plugin was reasonable: The forms module holds the directives not angulars2/angular2. Or did I miss something?

ShMcK commented 9 years ago

You're largely right. In the angular 2 dev file all routes are registered with System.js as follows:

System.register("angular2/angular2", ["angular2/src/core/application_common", 
"angular2/annotations", "angular2/change_detection", "angular2/core", "angular2/di", 
"angular2/directives", "angular2/http", "angular2/forms", "angular2/render"]

In other words, everything is packed in to angular2/angular2.

The development version loads all primary packages (except router). It's heavy, but while Angular 2 is under development I think simplicity is the way to go.

sclausen commented 9 years ago

Yes, I can agree to this approach of simplicity in development. But why does your original example work and your example with my updated angular2-typescript package not?

ShMcK commented 9 years ago

I'm really not sure. Can you share a link to your updated typescript package, and I'll try it out ?

sclausen commented 9 years ago

Sure :) meteor add sclausen:angular2-typescript and the repo is sclausen/meteor-angular2-typescript

ShMcK commented 9 years ago

That's great. I just tried it, and everything works.

Make sure in .meteor/packages you've added your package sclausen:angular2-typescript.

Question though: why target .ng.ts files instead of .ts ?

ShMcK commented 9 years ago

I use .ts files for Meteor as well, so I'm not sure why it needs a prefix.

sclausen commented 9 years ago

I treated .ng.ts and .ts. differently, because .ng.ts files are compiled with ModuleKind.System and .ts files with ModuleKind.CommonJS. It's currently not working well on the server side, because if you use TypeScript modules the transpiler produces require("module") and require doesn't work on meteor serverside.

ShMcK commented 9 years ago

Hmm... that's an interesting problem. What about shared .ts files ?

But then it seems the error only occurs with component modules containing imports & exports, which are all 100% client-side.

What if all files are treated with CommonJS?

sclausen commented 9 years ago

Yes, it only occurs on imports and exports. They're 100% client-side? What if I want to use a typescript module on the server?

I think we can't use CommonJS client-side, because angular2 uses System.js for managing the modules.

ShMcK commented 9 years ago

System.js is a universal module loader. It can load AMD or CommonJS.

But if you've found it wasn't working, something is wrong.

sclausen commented 9 years ago

If I use ModuleKind.System for every typescript file, the transpiler would wrap every file with a System.register. So you can't use for example a defined collection on the client side, because everything works different now with everything wrapped in System modules.

Parties = Mongo.Collection("parties");

will be transpiled to

(function(){System.register("model/parties.ts",[], function(exports_1) {
    return {
        setters:[],
        execute: function() {
            Parties = new Mongo.Collection("parties");
        }
    }
});

})();

And CommonJS modules don't work on the server, because I'll get the error ReferenceError: require is not defined

Maybe I should work on a small repo with different approaches to reproduce these errors.

ShMcK commented 9 years ago

Gotcha. Then .ng.ts files may our working solution. Thank you.

Once the solution is on Atmosphere, I'll add your solution to the tutorial.

Lots of thanks!

sclausen commented 9 years ago

Its already on atmosphere under sclausen:angular2-typescript, but I currently don't know if I should remove server side typescript completely, because of this module issue :-/ What do you think?

ShMcK commented 9 years ago

Give me a day with it, and I'll get back to you tomorrow. So far it looks promising.

sclausen commented 9 years ago

Sure, thanks!

Cirych commented 9 years ago

Maybe it possible to use similar approach, like in Universe Modules.

Cirych commented 9 years ago

I'm have no eloquence, so sorry for following whishes.

For learning ang2 and trying demos I'm using IDE (VSCode). For intellisense and transpiling we need .d.ts files, but no need they after. So meteor must ignore they, when processing source.

Maybe it was my error - offer to rename .ng.ts to .js in handler.js, but while it's working on client side, it's give error in IDE, cause module names. See no way now... Sorry ))) Sure, there are more complex way in Universe Modules, or in future Meteor release, with ES6 support.

ShMcK commented 9 years ago

Universal Modules can work for using ES2015 with Angular2. Good idea.

@sclausen, your solution works great except one problem. When I run server-side TypeScript it breaks. Is it possible to filter modules and process them in two different ways, .ng.ts files for Angular, and .ts files for the rest? That would be the go to solution.

ShMcK commented 9 years ago

Just a check: can you ensure that the --emitDecoratorMetadata flag is turned on for the TypeScript compiler. I'm having trouble with Dependency Injection in Angular 2, and I've heard this is a possible cause.

sclausen commented 9 years ago

Hey @ShMcK, I already treat .ng.ts differently than .ts and stumbled upon problems on the server side too, but currently I don't know how to treat plain .ts correctly :-/

sclausen commented 9 years ago

@ShMcK I took a peak at Universal Modules and piping typescript into that isn't trivial. There is also an issue for this task vazco/universe-modules/issues/17

I added the transpiler options emitDecoratorMetadata and experimentalDecorators. Now i've got problems with shared code between client and server, more precisely with collections.

I get the following diagnostics message after transpiling model/parties.ts

{
  "messageText": "Option 'isolatedModules' can only be used when either option'--module' is provided or option 'target' is 'ES6' or higher.",
  "category": 1,
  "code": 5047
}

the code of parties.ts is the common

Parties = new Mongo.Collection("parties");

The code is in the master branch of my plugin, but not published on atmosphere yet.

I'm slowly growing desperate with this plugin :-/

ShMcK commented 9 years ago

I'll take a look at it tomorrow. Busy day today! But I'd like to thank you for your efforts. Solid job!

But overall I think it can work. It will have to compile typescript twice based on the filter:

.ng.ts gets emitDecoratorMetadata, experimentalDecorators, modules, etc. .ts normal typeScript

This is all experimental at this point, but it looks promising.

ShMcK commented 9 years ago

I thought I had it working, but no luck. Handling the files differently just resulted in the same "require" error in the browser console as before. Any chance the typescript configuration is wrong here?

var SETTINGS = {
  ngTs: {
    module: typescript.ModuleKind.System,
    emitDecoratorMetadata: true,
    experimentalDecorators: true
  },
  ts: {
    module: typescript.ModuleKind.System
  }
};
Urigo commented 9 years ago

What about using this package: https://github.com/meteor-typescript/meteor-typescript-compiler for handling regular .ts files? (sorry if I'm missing something, just a fast response..)

dalcib commented 9 years ago

meteor-typescript-compiler is outdated (typescript: "1.4.1") and it doesn't work with Typescript external modules, that allow use import statement, used to load Angular2 in Typescript.

Urigo commented 9 years ago

yes... I thought (or hoped) that they will update but I guess not...

ShMcK commented 9 years ago

I've posted an issue on the TypeScript repo for assistance. https://github.com/Microsoft/TypeScript/issues/4269

sclausen commented 9 years ago

I don't think it's a good idea to wrap every meteor file in a system.js module. Therefor you have to change everything you built with meteor to use the imports and every file has to be a self contained module. You can declare collections on server&client side simultaneously simply like following code:

MongCollection = new Mongo.Collection("collection-name");

In Meteor you now can use this Collection with window.MongoCollection on the client side and with MongoCollection on the server side. This would be nasty, if you have to change the normal meteor workflow and use imports everywhere, beside I didn't got this case to work at all.

I think its also a problem we use meteor, because the sourceHandler from meteor incrementally compiles every file and not all at once. Therefor the typescript compiler can't know if there exist any modules. The typescript compiler allows to pass an array of files to the program like this:

var program = ts.createProgram(fileNames, options);

So maybe Plugin.registerSourceHandler is the wrong approach and we need Plugin.registerCompiler as described here: meteor/meteor/wiki/Build-Plugins-API

ShMcK commented 9 years ago

Another possible short term solution would be to replace the typescript compiler, and instead just configure System.js.

The upside is it would only run client-side.

The down side is, this would all be run-time compiled and not production suitable.

<script>
 System.import('client/app');
  System.config({
    transpiler: 'typescript',
       typescriptOptions: {
           module: typescript.ModuleKind.System,
           emitDecoratorMetadata: true,
           experimentalDecorators: true
    }
  });
</script>

The configuration could be installed with the setup as default. Users could enter a settings option such as:

Angular2.transpiler = "ts"

Other options could include "babel", or "es5", with corresponding configurations.

I'll look into Plugin.registerCompiler as well. I'm hoping we can get to a better usable solution soon.

sclausen commented 9 years ago

I'm currently working with the meteor version PLUGINS-PREVIEW@2 and its compiling well regarding the server-side part with modules and stuff. Emitting errors if declarations are messed and so on. Maybe I'll get a working version where one of my last approaches compiles the frontend well and the backend will be compiled with the other approach I'm evaluating now.