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

Re-Simplify external modules #85

Closed
basarat opened this issue Mar 16, 2014 · 11 comments · Fixed by #87
Closed

Re-Simplify external modules #85

basarat opened this issue Mar 16, 2014 · 11 comments · Fixed by #87

Comments

@basarat
Copy link
Member

basarat commented Mar 16, 2014

Using the index option does help for large projects. But for quickly getting off the ground there was too much effort required in configuration. See : Deprecation Reasons. The new plan below is simple to understand, and requires no task configuration.

New Version:

Updated based on final code commited

User types in

///ts:import=filename

grunt-ts comes along, notices this and replaces it with

///ts:import=filename
import filename = require('../path/to/filename'); ///ts:import:generated

User types in

///ts:import=foldername

grunt-ts comes along, notices this and replaces it with

///ts:import=foldername
import filename = require('../path/to/foldername/filename'); ///ts:import:generated
import anotherfile = require('../path/to/foldername/deeper/anotherfile'); ///ts:import:generated
...

If a folder has an index.ts inside of it then we do not import the entire folder and only import index.ts. i.e :

///ts:import=foldername
import foldername = require('../path/to/foldername/index'); ///ts:import:generated

You can also use grunt-ts to create an index file for you.

///ts:export=foldername
export import filea= require('../path/to/foldername/filea'); ///ts:export:generated
// so on ... 

To see a fullstack application that uses this : https://github.com/basarat/demo-fullstack/blob/master/src/server/routes/index.ts

@basarat basarat added this to the External Modules milestone Mar 16, 2014
@nycdotnet
Copy link
Contributor

Where do propose to get the imported variable name from... the file name of the TS file as in the right-most element in the require path?

Can you think of a good way to have grunt-ts allow for aliasing a require? I use requirejs in my project at runtime and In this case, I would like for the relative paths to exist in my config for require, but the libraries themselves to just be aliased simply.

For example, something like this would be amazing:

///import={path:"../path/to/filename",alias:"MyLibrary",varName:"Thing"}

grunt-ts might change the TypeScript to:

///import={path:"../path/to/filename",alias:"MyLibrary",varName:"Thing"}
import Thing = require("../path/to/filename");

And then perhaps edit the emitted JS in a post-build step from this:

define(["require", "exports", "../path/to/filename"], function(require, exports, Thing) { 

to this:

define(["require", "exports", "MyLibrary"], function(require, exports, Thing) { 

To ask a silly question - I'm assuming that it's impossible for this to work in Visual Studio, right?

@basarat
Copy link
Member Author

basarat commented Mar 16, 2014

Can you think of a good way to have grunt-ts allow for aliasing a require

You suggestion is actually quite good. But not going to be a part of this, Reason: It will only cater for requirejs whereas the plan here is to simplify for both amd/commonjs. PS: grunt-ts doesn't know about the generated js (yet).

varName:"Thing"
path:"../path/to/filename"

I am trying to minimize keystrokes / config. People can rename their file to be Thing if that is what they want :) PS: the objective is to generate the path for the user. We figure out the path from expanding the src glob from our gruntfile and matching the filename.

To ask a silly question - I'm assuming that it's impossible for this to work in Visual Studio, right?

Possible. But no one has done tight integration yet. I suspect microsoft will add grunt support to visual studio at some point, since it has become a vital part of javascript development. Ofcourse if you have grunt-ts running in the background (and configured for watching) and modify the file in visual studio, the file should update before your eyes in visual studio.

///import=

interesting you used = instead of :. I prefer colon since it reminded me of script:src expansion in emmet.io (zen coding). Plus = is too far away from the home row. And using = immediately makes me want to do import = filename due to my mental training for space around assignments

@nycdotnet Thank you for reviewing this!

@nycdotnet
Copy link
Contributor

This is all great stuff. Thanks for considering it. The idea that it would be good to have TypeScript be able to alias modules in the emitted JS is https://typescript.codeplex.com/workitem/2317 I would love for you to vote on it/promote it if you agree with the idea.

I find it fascinating that you are optimizing for inches on the keyboard. You must develop code at an extraordinary pace if your keyboard layout affects your design choices. Can't wait for your new external modules video.

@basarat
Copy link
Member Author

basarat commented Mar 17, 2014

I would love for you to vote on it/promote it if you agree with the idea.

it shouldn't generate ../Scripts/typings/MyIdea/MyIdea.d using generated definitions for external modules feels broken to me. The issue is in original declaration code generation. It should generate a .d.ts that you ///<reference import (to get the type info) and then do a require('whatever') to load the js. This is similar to how you can require e.g. underscore.

I find it fascinating that you are optimizing for inches on the keyboard.

Not at all. I was only justifying my probably bad naming decision :)

Its "probably" (not final till I commit the code) going to end up as:

///ts:import=filename|foldername

so that it namespaced ///ts: to allow for discoverability in code reviews.

@basarat
Copy link
Member Author

basarat commented Mar 18, 2014

@nycdotnet deployed to npm as 1.8.2 with the syntax mentioned in this (updated) comment : #85 (comment)

@basarat
Copy link
Member Author

basarat commented Mar 18, 2014

for anyone interested you can see fast compile + external modules code generation working in this code sample : https://github.com/basarat/demo-fullstack/tree/master/src

I will document it better once I get time

@basarat
Copy link
Member Author

basarat commented Mar 18, 2014

Its out the door. And I already think it needs more configurability.

e.g. When we want to "actually" load all the files in a folder it will not work since a simple import does not do any codegen unless the variable is actually used.

So I'll bump the major version and introduce :

///ts:import={name:"Thing",export:true}

The export option will generate
export import for you. This will also make it easier for you to author index files.

New Feature tracking : #88

@basarat
Copy link
Member Author

basarat commented Mar 19, 2014

I tried it. It makes for a cumbersome syntax (inline json in a comment) and didn't like it. So decided to make it a different codegen task (calling these transforms). Updated #88 accordingly. So this task (import transform) will remain valid (and useful)

@nycdotnet
Copy link
Contributor

Thanks for your hard work on this. I'm glad that you feel comfortable iterating on this if you try and dislike the first outcome.


From: Basarat Ali Syedmailto:[email protected]
Sent: ‎3/‎19/‎2014 4:36 AM
To: grunt-ts/grunt-tsmailto:[email protected]
Cc: Steve Ognibenemailto:[email protected]
Subject: Re: [grunt-ts] Re-Simplify external modules (#85)

I tried it. It makes for a cumbersome syntax (inline json in a comment) and didn't like it. So decided to make it a different codegen task (calling these transforms). Updated #88 accordingly. So this task (import transform) will remain valid (and useful)


Reply to this email directly or view it on GitHub:
#85 (comment)

@nexussays
Copy link
Member

Have you put any more thought into providing a syntax to allow aliasing of the generated imports?

@basarat
Copy link
Member Author

basarat commented Aug 25, 2014

Have you put any more thought into providing a syntax to allow aliasing of the generated imports?

Nope. You are welcome to open a new issue with this subject if you want.

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

Successfully merging a pull request may close this issue.

3 participants