Skip to content
This repository has been archived by the owner on Jul 29, 2024. It is now read-only.

feat(typescript): adding typescript to protractor #2902

Merged
merged 1 commit into from
Feb 16, 2016

Conversation

cnishina
Copy link
Member

@cnishina cnishina commented Feb 4, 2016

Converting a 3 files over to typescript.

On the prepublish step, it will download the typings, transpile the files with tsc, and move the transpiled javascript files from the built directory to the lib.

Also adding scripts to package.json for npm run tsc and npm run tsc:w for transpiling.

@cnishina
Copy link
Member Author

cnishina commented Feb 4, 2016

The linter was checking the generated javascript files from typescript. Removing the check to lib as a "fix".

@@ -2,4 +2,11 @@

process.env.NODE_ENV = process.env.NODE_ENV || 'test';

// Transpile code to javascript prior to running javascript
var spawn = require('child_process').spawnSync;
var transpile = spawn('node_modules/typescript/bin/tsc');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the strategy of compiling files in the client is very different from what we do elsewhere. why not ship compiled .js files to the client?

@cnishina cnishina force-pushed the typescript branch 2 times, most recently from 825cc59 to 9e09f01 Compare February 5, 2016 03:07
@@ -0,0 +1,4 @@
# TODO: remove this file when all files have been replaced with typescript
configParser.js

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you still need this? seems like the file is now removed by this change

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still needed. On the prepublish step, the transpiled javascript files are being copied into the lib folder.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, because the published app references lib/file.js in package.json
maybe it should reference built/file.js instead, like Julie suggests

@cnishina cnishina force-pushed the typescript branch 2 times, most recently from a390b59 to 63162f5 Compare February 11, 2016 00:32
@cnishina
Copy link
Member Author

Added jslint check back for lib/ since transpiled files are now in built/

@cnishina cnishina force-pushed the typescript branch 3 times, most recently from 8f892e2 to 999bf07 Compare February 13, 2016 00:12
@cnishina cnishina assigned juliemr and unassigned cnishina Feb 13, 2016
@@ -12,3 +12,6 @@ xmloutput*
npm-debug.log

*.swp

typings/
built/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

built/ is already at the top from the other PR, remove this line

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@juliemr
Copy link
Member

juliemr commented Feb 16, 2016

Just a couple cleanup comments, otherwise looks good!

@juliemr juliemr assigned cnishina and unassigned juliemr Feb 16, 2016
@cnishina cnishina force-pushed the typescript branch 7 times, most recently from 93de546 to 78f7c91 Compare February 16, 2016 21:34
Converting a 3 files over to typescript.

Adding an `npm prepublish` step that will use gulp to download the typings, transpile the files with tscto the built/ directory and copy the rest of the javascript files from lib/ to the built/ folder.

Also adding scripts to package.json for `npm run tsc` and `npm run tsc:w` for transpiling help.
@cnishina cnishina merged commit 9608201 into angular:master Feb 16, 2016
@cnishina
Copy link
Member Author

Reference feature request: #1203

@cnishina cnishina mentioned this pull request Jun 27, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants