feat(tsc-wrapped): Support of vinyl like config file was added #13987

jolly-roger · 2017-01-18T13:45:53Z

This feature was implemented in order to provide easier way of use in gulp

Please check if the PR fulfills these requirements

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (check one with "x")

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

What is the new behavior?

Does this PR introduce a breaking change? (check one with "x")

[ ] Yes
[ ] No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

alexeagle · 2017-01-18T23:26:39Z

tools/@angular/tsc-wrapped/src/main.ts

@@ -23,11 +23,16 @@ export type CodegenExtension =
        Promise<void>;

 export function main(
-    project: string, cliOptions: CliOptions, codegen?: CodegenExtension,
+    project: any, cliOptions: CliOptions, codegen?: CodegenExtension,


instead of any, can it be string|{path:string}, or can you introduce an interface that structurally matches like VinylFile?

alexeagle · 2017-01-18T23:26:57Z

tools/@angular/tsc-wrapped/src/tsc.ts

@@ -97,7 +97,7 @@ export class Tsc implements CompilerInterface {

  constructor(private readFile = ts.sys.readFile, private readDirectory = ts.sys.readDirectory) {}

-  readConfiguration(project: string, basePath: string, existingOptions?: ts.CompilerOptions) {
+  readConfiguration(project: any, basePath: string, existingOptions?: ts.CompilerOptions) {


again, can you avoid any?

again, agree

alexeagle · 2017-01-18T23:29:00Z

tools/@angular/tsc-wrapped/test/main.spec.ts

+        .then(() => {
+          const out = readOut('js');
+          // No helpers since decorators were lowered
+          expect(out).not.toContain('__decorate');


I don't think we should repeat all the assertions here - you just need to assert enough to be sure the vinyl file path and contents were used. Just check that some of your config was used

alexeagle · 2017-01-18T23:31:51Z

tools/@angular/tsc-wrapped/test/tsc.spec.ts

@@ -7,12 +7,11 @@
 */

 import * as ts from 'typescript';
-import {Tsc} from '../src/tsc';
+import {Tsc, tsc} from '../src/tsc';


this import is now shadowed by the variable on line 25, can you avoid that?

alexeagle

thanks looking close

alexeagle · 2017-01-19T21:42:44Z

tools/@angular/tsc-wrapped/src/main.ts

-    if (fs.lstatSync(project).isFile()) {
-      projectDir = path.dirname(project);
+    // project is vinyl like file object
+    if ((project as VinylFile).path) {


can you add isVinylFile to ./vinyl that returns project is VinylFile
See User-Defined Type Guards on https://www.typescriptlang.org/docs/handbook/advanced-types.html

Then just call isVinylFile instead of casting here

Aha, type guard - nice. Sure will add.

alexeagle · 2017-01-19T21:43:14Z

tools/@angular/tsc-wrapped/src/tsc.ts

    this.basePath = basePath;

    // Allow a directory containing tsconfig.json as the project value
    // Note, TS@next returns an empty array, while earlier versions throw
    try {
-      if (this.readDirectory(project).length > 0) {
-        project = path.join(project, 'tsconfig.json');
+      if (this.readDirectory(project as string).length > 0) {


use !isVinylFile to narrow the type instead of casting

alexeagle · 2017-01-20T16:48:57Z

tools/@angular/tsc-wrapped/src/main.ts

+      projectDir = path.dirname(project.path);
+    }
+    // project is path to project file
+    else if (fs.lstatSync(project as string).isFile()) {


you shouldn't need a cast here anymore - the project type will have the VinylFile narrowed out since isVinylFile is known to be false in this else clause

Wow, very tricky, but now I know it :)

alexeagle · 2017-01-20T16:49:31Z

tools/@angular/tsc-wrapped/src/tsc.ts

    this.basePath = basePath;

    // Allow a directory containing tsconfig.json as the project value
    // Note, TS@next returns an empty array, while earlier versions throw
    try {
-      if (this.readDirectory(project).length > 0) {
-        project = path.join(project, 'tsconfig.json');
+      if (!isVinylFile(project) && this.readDirectory(project as string).length > 0) {


same here I believe

alexeagle · 2017-01-20T16:49:43Z

tools/@angular/tsc-wrapped/src/tsc.ts

+      }
+      // project is path to project file
+      else {
+        return ts.readConfigFile(project as string, this.readFile);


alxhub · 2017-01-20T22:29:59Z

I'm getting conflicts when rebasing this on master, please rebase and fix them and then I can merge it. Thanks, @jolly-roger

This feature was implemented in order to provide easier way of use in gulp

Test was fixed.

Test case config was fixed.

Code was corrected according to the remarks

Code formatting was corrected

Type guard was introduced for VinylFile

Cleanup.

jolly-roger · 2017-01-22T12:19:31Z

@alxhub, Rebasing and resolving conflicts are done.
Thanks a lot.

…ar#13987) This feature was implemented in order to provide easier way of use in gulp

angular-automatic-lock-bot · 2019-09-10T16:20:51Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Jan 18, 2017

mhevery assigned alexeagle Jan 18, 2017

alexeagle suggested changes Jan 18, 2017

View reviewed changes

alexeagle suggested changes Jan 19, 2017

View reviewed changes

alexeagle approved these changes Jan 20, 2017

View reviewed changes

alxhub added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: merge The PR is ready for merge by the caretaker labels Jan 20, 2017

alxhub assigned jolly-roger and unassigned alexeagle Jan 20, 2017

jolly-roger added 7 commits January 22, 2017 13:48

feat(tsc-wrapped): Support of vinyl like config file was added

e3b6091

This feature was implemented in order to provide easier way of use in gulp

feat(tsc-wrapped): Support of vinyl like config file was added

ede67b9

Test was fixed.

feat(tsc-wrapped): Support of vinyl like config file was added

f5bf76e

Test case config was fixed.

feat(tsc-wrapped): Support of vinyl like config file was added

7b3f252

Code was corrected according to the remarks

feat(tsc-wrapped): Support of vinyl like config file was added

ebc2a90

Code formatting was corrected

feat(tsc-wrapped): Support of vinyl like config file was added

db14e8d

Type guard was introduced for VinylFile

feat(tsc-wrapped): Support of vinyl like config file was added

0f1a713

Cleanup.

jolly-roger force-pushed the tsc-wrapper-config-obj branch from 5d47856 to 0f1a713 Compare January 22, 2017 11:56

alexeagle removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Jan 23, 2017

alexeagle modified the milestone: rg Jan 23, 2017

alexeagle added the action: merge The PR is ready for merge by the caretaker label Jan 23, 2017

alxhub merged commit 0c7726d into angular:master Jan 24, 2017

jolly-roger deleted the tsc-wrapper-config-obj branch January 25, 2017 09:09

juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017

feat(tsc-wrapped): Support of vinyl like config file was added (angul…

3c72f39

…ar#13987) This feature was implemented in order to provide easier way of use in gulp

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(tsc-wrapped): Support of vinyl like config file was added #13987

feat(tsc-wrapped): Support of vinyl like config file was added #13987

jolly-roger commented Jan 18, 2017

alexeagle Jan 18, 2017

jolly-roger Jan 19, 2017

alexeagle Jan 18, 2017

jolly-roger Jan 19, 2017 •

edited

Loading

alexeagle Jan 18, 2017

jolly-roger Jan 19, 2017

alexeagle Jan 18, 2017

jolly-roger Jan 19, 2017

alexeagle left a comment

alexeagle Jan 19, 2017

jolly-roger Jan 20, 2017

alexeagle Jan 19, 2017

jolly-roger Jan 20, 2017

alexeagle Jan 20, 2017

jolly-roger Jan 20, 2017

alexeagle Jan 20, 2017

jolly-roger Jan 20, 2017

alexeagle Jan 20, 2017

jolly-roger Jan 20, 2017

alxhub commented Jan 20, 2017

jolly-roger commented Jan 22, 2017

angular-automatic-lock-bot bot commented Sep 10, 2019

feat(tsc-wrapped): Support of vinyl like config file was added #13987

feat(tsc-wrapped): Support of vinyl like config file was added #13987

Conversation

jolly-roger commented Jan 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jolly-roger Jan 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexeagle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alxhub commented Jan 20, 2017

jolly-roger commented Jan 22, 2017

angular-automatic-lock-bot bot commented Sep 10, 2019

jolly-roger Jan 19, 2017 •

edited

Loading