Typescript example #13

Closed
wants to merge 10 commits into
from

Conversation

Projects
None yet
3 participants
@marwijn

marwijn commented Dec 3, 2012

Hello Rob,

I've added an hellots example which uses typescript.
If you like it you can pull it, if not any comments are appreciated.
As I was unable to directly call a module I've added a getViewModel function to the module, for which I made a special check.

Kind regards,
Marwijn.

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Dec 4, 2012

Member

Cool stuff. One of the problems is that TypeScript doesn't allow for functions as exports, which is why you had to add the modification in the composition module. That's a flaw in TypeScript and in the ES6 module spec since those using AMD modules do this all the time. I'm hopeful that it will get fixed. In the mean time, I'm going to take a look at your code more thoroughly and consider if there isn't a way I can make an improvement in the composition module similar to what you've done, but perhaps more flexible for other uses. We'll see. Thanks!

Member

EisenbergEffect commented Dec 4, 2012

Cool stuff. One of the problems is that TypeScript doesn't allow for functions as exports, which is why you had to add the modification in the composition module. That's a flaw in TypeScript and in the ES6 module spec since those using AMD modules do this all the time. I'm hopeful that it will get fixed. In the mean time, I'm going to take a look at your code more thoroughly and consider if there isn't a way I can make an improvement in the composition module similar to what you've done, but perhaps more flexible for other uses. We'll see. Thanks!

@marwijn

This comment has been minimized.

Show comment
Hide comment
@marwijn

marwijn Dec 4, 2012

Hello Rob,

Thanks for commenting. The more I'm working with the export functionality in TypeScript the more I wonder if this already mature enough to use. In a previous attempt (https://github.com/marwijn/Durandal/tree/352023ce588e87113cc6e534e9e7823efe358625) to use TypeScript with durandel I created the shell module as shown below:

/// <reference path="..\..\dts\require-2.1.d.ts" />
/// <reference path="..\..\dts\knockout-2.2.d.ts" />

define(['durandal/app'], (app) => {

    return new Shell(app) }
)

class Shell {
        displayName: KnockoutObservableString;
        name: KnockoutObservableString;
        canSayHello: KnockoutObservableBool;
        app: any;

        constructor (app : any) {
            this.app = app;
            this.name = ko.observable();
            this.displayName = ko.observable();
            this.canSayHello = ko.computed(()=> this.name() ? true : false);
        }

        public sayHello()
        { 
            this.app.showMessage("Hello " + this.name(), "Greetings");
        }
    }

This did not require any changes to Durandel. Morever it is also possible to add a type to app, using a d.ts file. I was unable to get that working with the module import/export functionality. Downside is that this gives a dependency on require and it is not possible to compile modules to CommonJs anymore.

What is your opinion. I would like to go forward with adding typescript examples However I'm unsure which approach to take.

Kind regards,
Marwijn.

PS. I started this as a pull request, but I think it is better not to pull till it is clear what road to take.

marwijn commented Dec 4, 2012

Hello Rob,

Thanks for commenting. The more I'm working with the export functionality in TypeScript the more I wonder if this already mature enough to use. In a previous attempt (https://github.com/marwijn/Durandal/tree/352023ce588e87113cc6e534e9e7823efe358625) to use TypeScript with durandel I created the shell module as shown below:

/// <reference path="..\..\dts\require-2.1.d.ts" />
/// <reference path="..\..\dts\knockout-2.2.d.ts" />

define(['durandal/app'], (app) => {

    return new Shell(app) }
)

class Shell {
        displayName: KnockoutObservableString;
        name: KnockoutObservableString;
        canSayHello: KnockoutObservableBool;
        app: any;

        constructor (app : any) {
            this.app = app;
            this.name = ko.observable();
            this.displayName = ko.observable();
            this.canSayHello = ko.computed(()=> this.name() ? true : false);
        }

        public sayHello()
        { 
            this.app.showMessage("Hello " + this.name(), "Greetings");
        }
    }

This did not require any changes to Durandel. Morever it is also possible to add a type to app, using a d.ts file. I was unable to get that working with the module import/export functionality. Downside is that this gives a dependency on require and it is not possible to compile modules to CommonJs anymore.

What is your opinion. I would like to go forward with adding typescript examples However I'm unsure which approach to take.

Kind regards,
Marwijn.

PS. I started this as a pull request, but I think it is better not to pull till it is clear what road to take.

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Dec 11, 2012

Member

I have some ideas around enabling you to patch Durandal easily in order to solve the Class moduleId problem. Give me a little more time to think it through...

Member

EisenbergEffect commented Dec 11, 2012

I have some ideas around enabling you to patch Durandal easily in order to solve the Class moduleId problem. Give me a little more time to think it through...

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Dec 12, 2012

Member

Further investigation revealed that the recently updated version of the ES6 modules spec fixes the issue related to not being able to export a constructor function as a module. So, my guess is that, since TypeScript is attempting to follow the ES6 language, Microsoft will probably be fixing that in their implementation. In that case, the issues described above will go away entirely.

In light of that, I'm not sure what to do in the man time. I don't want to make any alterations to Durandal for problems in other languages which are likely to get fixed.

Do you have any thoughts on this?

Member

EisenbergEffect commented Dec 12, 2012

Further investigation revealed that the recently updated version of the ES6 modules spec fixes the issue related to not being able to export a constructor function as a module. So, my guess is that, since TypeScript is attempting to follow the ES6 language, Microsoft will probably be fixing that in their implementation. In that case, the issues described above will go away entirely.

In light of that, I'm not sure what to do in the man time. I don't want to make any alterations to Durandal for problems in other languages which are likely to get fixed.

Do you have any thoughts on this?

@marwijn

This comment has been minimized.

Show comment
Hide comment
@marwijn

marwijn Dec 13, 2012

I fully agree that one should not make alteration to a framework as Durandal for problems that are probably get fixed soon.
One thing I was thinking is to change root to allow it to accept something like:

app.setRoot('samples/masterDetail/shell#SomeClass');

This would also need a change in requirejs.onResourceLoad to set the moduleId on the loaded classes.
I can't fully grab the implications of this for the complete application, however it feels usefull to me that a module can contain more than a single class. I'm still experimenting with this, and especially wonder about how to resolve the view for the viewModel in this case. Maybe create a view with name shell.SomeClass.html ?

To me this seems less hackish than the original approach I was taking. What is your opinion on this, could this be a good addition to Durandal ?

Kind regards,
Marwijn.

marwijn commented Dec 13, 2012

I fully agree that one should not make alteration to a framework as Durandal for problems that are probably get fixed soon.
One thing I was thinking is to change root to allow it to accept something like:

app.setRoot('samples/masterDetail/shell#SomeClass');

This would also need a change in requirejs.onResourceLoad to set the moduleId on the loaded classes.
I can't fully grab the implications of this for the complete application, however it feels usefull to me that a module can contain more than a single class. I'm still experimenting with this, and especially wonder about how to resolve the view for the viewModel in this case. Maybe create a view with name shell.SomeClass.html ?

To me this seems less hackish than the original approach I was taking. What is your opinion on this, could this be a good addition to Durandal ?

Kind regards,
Marwijn.

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Dec 13, 2012

Member

That's a really interesting idea. With respect to onResourceLoad, do you have any strategy for how you would detect a module that had class members like this? Would you just for/in the object and look for all keys of Function type? or is there another way that's more efficient?

Member

EisenbergEffect commented Dec 13, 2012

That's a really interesting idea. With respect to onResourceLoad, do you have any strategy for how you would detect a module that had class members like this? Would you just for/in the object and look for all keys of Function type? or is there another way that's more efficient?

@marwijn

This comment has been minimized.

Show comment
Hide comment
@marwijn

marwijn Dec 13, 2012

Hello Rob,

I've commited a part of the idea I had. I didn't change the view resolution yet. So now when you have multiple classes in a typescript module they will all get the same view.
Further the example doesn't work fully yet, switching projects doesn't work. The helloTs sample does work. I've not been able to figure it out yet.

Kind regards,
Marwijn.

marwijn commented Dec 13, 2012

Hello Rob,

I've commited a part of the idea I had. I didn't change the view resolution yet. So now when you have multiple classes in a typescript module they will all get the same view.
Further the example doesn't work fully yet, switching projects doesn't work. The helloTs sample does work. I've not been able to figure it out yet.

Kind regards,
Marwijn.

marwijn added some commits Dec 13, 2012

Changed View Location
When using class inside a module, the view is expected to be in
<module>_<className>.html
@marwijn

This comment has been minimized.

Show comment
Hide comment
@marwijn

marwijn Dec 13, 2012

comments please.

marwijn commented Dec 13, 2012

comments please.

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Dec 14, 2012

Member

Let me take a look at it this afternoon. I'm not ignoring you...just really busy :)

Member

EisenbergEffect commented Dec 14, 2012

Let me take a look at it this afternoon. I'm not ignoring you...just really busy :)

@marwijn

This comment has been minimized.

Show comment
Hide comment

marwijn commented Dec 14, 2012

Relax !

marwijn added some commits Dec 14, 2012

Merge branch 'master' of https://github.com/EisenbergEffect/Durandal
Conflicts:
	app/durandal/composition.js
	app/durandal/viewLocator.js
	app/main.js
@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Dec 18, 2012

Member

Preliminary investigation looks good. I need to ponder it a bit more, but I think this would probably be a nice addition.

Member

EisenbergEffect commented Dec 18, 2012

Preliminary investigation looks good. I need to ponder it a bit more, but I think this would probably be a nice addition.

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Jan 6, 2013

Member

Still pondering. I think I'm going to incorporate the code into Durandal. However, regarding the sample. I think it would be better to create a separate repository for samples that are specific to TypeScript or other transpiled languages. Does that make sense?

Member

EisenbergEffect commented Jan 6, 2013

Still pondering. I think I'm going to incorporate the code into Durandal. However, regarding the sample. I think it would be better to create a separate repository for samples that are specific to TypeScript or other transpiled languages. Does that make sense?

@evanlarsen

This comment has been minimized.

Show comment
Hide comment
@evanlarsen

evanlarsen Jan 7, 2013

Contributor

Here's an alternate way. If your folder structure is like this:

  • modules/
    • page.html
    • page.ts
  • libs/
    • durandal/
      • system.js
      • composition.js
      • ....
      • durandal.d.ts
durandal.d.ts
declare module "libs/durandal/system" {
    export var guid: () => string;
}
declare module "libs/durandal/dom" {
    export var getElementById: (id: string) => HTMLElement;
}
page.ts
/// <reference path='../libs/durandal/durandal.d.ts' />
import system = module('libs/durandal/system');
import dom = module('libs/durandal/dom');

var x = system.guid();
var ele = dom.getElementById('header');
Contributor

evanlarsen commented Jan 7, 2013

Here's an alternate way. If your folder structure is like this:

  • modules/
    • page.html
    • page.ts
  • libs/
    • durandal/
      • system.js
      • composition.js
      • ....
      • durandal.d.ts
durandal.d.ts
declare module "libs/durandal/system" {
    export var guid: () => string;
}
declare module "libs/durandal/dom" {
    export var getElementById: (id: string) => HTMLElement;
}
page.ts
/// <reference path='../libs/durandal/durandal.d.ts' />
import system = module('libs/durandal/system');
import dom = module('libs/durandal/dom');

var x = system.guid();
var ele = dom.getElementById('header');
@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Jan 9, 2013

Member

OH! I didn't realize you had started working on a definition file for durandal. That's really cool!

Member

EisenbergEffect commented Jan 9, 2013

OH! I didn't realize you had started working on a definition file for durandal. That's really cool!

@evanlarsen

This comment has been minimized.

Show comment
Hide comment
@evanlarsen

evanlarsen Jan 10, 2013

Contributor

There's probably a better way to do it, but this gets the job done :)

Contributor

evanlarsen commented Jan 10, 2013

There's probably a better way to do it, but this gets the job done :)

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Jan 18, 2013

Member

Here's an idea. What if we author a custom module, under the plugins folder that would patch up durandal to work with TypeScript. So like this:

var typeScript = require('plugins/typeScript');
typeScript.install();

That could money path the appropriate durandal modules until TypeScript fixes their implementation. Thoughts?

Member

EisenbergEffect commented Jan 18, 2013

Here's an idea. What if we author a custom module, under the plugins folder that would patch up durandal to work with TypeScript. So like this:

var typeScript = require('plugins/typeScript');
typeScript.install();

That could money path the appropriate durandal modules until TypeScript fixes their implementation. Thoughts?

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Jan 18, 2013

Member

Sorry, I meant it could "money patch" durandal.

Member

EisenbergEffect commented Jan 18, 2013

Sorry, I meant it could "money patch" durandal.

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Jan 18, 2013

Member

Dang it...still can't type..."monkey patch"...that's it.

Member

EisenbergEffect commented Jan 18, 2013

Dang it...still can't type..."monkey patch"...that's it.

@evanlarsen

This comment has been minimized.

Show comment
Hide comment
@evanlarsen

evanlarsen Jan 18, 2013

Contributor

Personally, I wouldn't. Just keep on going the way you have been going. I'm using your library just fine with typescript. I just ran into that 1 problem with the transitions.

Typescript just has that 1 issue where you can't return a function from an AMD and pretty much transitions is the only place in durandal that's expected. Its not a big deal.

I'm extremely happy with durandal and so far its working perfectly with typescript. So, I wouldn't worry about making some Typescript specific patch.

Contributor

evanlarsen commented Jan 18, 2013

Personally, I wouldn't. Just keep on going the way you have been going. I'm using your library just fine with typescript. I just ran into that 1 problem with the transitions.

Typescript just has that 1 issue where you can't return a function from an AMD and pretty much transitions is the only place in durandal that's expected. Its not a big deal.

I'm extremely happy with durandal and so far its working perfectly with typescript. So, I wouldn't worry about making some Typescript specific patch.

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Jan 22, 2013

Member

I'm going to close this issue. I'd rather not make a change in the core based on the current state of TypeScript. Rather, I'd prefer a plugin that re-configures durandal for this case. But, it will get fixed in TypeScript before v1.0 I confirmed that by checking their roadmap. Also Luke Hoban responded to me via codeplex to say they have every intent to fix it.

Member

EisenbergEffect commented Jan 22, 2013

I'm going to close this issue. I'd rather not make a change in the core based on the current state of TypeScript. Rather, I'd prefer a plugin that re-configures durandal for this case. But, it will get fixed in TypeScript before v1.0 I confirmed that by checking their roadmap. Also Luke Hoban responded to me via codeplex to say they have every intent to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment