Skip to content

Update deps#550

Merged
ivogabe merged 4 commits intoivogabe:masterfrom
rogierschouten:update-deps
Dec 27, 2017
Merged

Update deps#550
ivogabe merged 4 commits intoivogabe:masterfrom
rogierschouten:update-deps

Conversation

@rogierschouten
Copy link
Copy Markdown

A number of gulp plugins relies (directly or indirectly) on a very old version of 'vinyl'. This causes problems with differently-shaped vinyl objects going through the gulp pipelines. gulp-typescript relies on an old version of vinyl-fs which relies on this older version of vinyl.

Unfortunatly I wasn't able to run the tests, and I think I'm not running the test command correctly. If something is wrong please tell me what I need to do to run the tests. I got this output:

$ npm test

> gulp-typescript@3.2.3 test C:\Source\github\gulp-typescript
> gulp

module.js:529
    throw err;
    ^

Error: Cannot find module './typescript/2.3'
    at Function.Module._resolveFilename (module.js:527:15)
    at Function.Module._load (module.js:476:23)
    at Module.require (module.js:568:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (C:\Source\github\gulp-typescript\gulpfile.js:82:10)
    at Module._compile (module.js:624:30)
    at Object.Module._extensions..js (module.js:635:10)
    at Module.load (module.js:545:32)
    at tryModuleLoad (module.js:508:12)
    at Function.Module._load (module.js:500:3)
npm ERR! Test failed.  See above for more details.

@phated
Copy link
Copy Markdown

phated commented Dec 22, 2017

This module needs to move off gulp-util to fix the gulp 4 issues.

@ivogabe
Copy link
Copy Markdown
Owner

ivogabe commented Dec 23, 2017

Thanks for reporting and for the PR. I wasn't aware of that. The tests seems to be failed as TypeScript made their type definitions more strict: they changed Diagnostic[] by ReadonlyArray<Diagnostic> and gulp-typescript should thus be updated too. TypeScript usually has some breaking changes every release like this one, so I always lock the version of TypeScript to a specific one. Can you change that?

You should install the git submodules to run the tests (git submodule update --init).

Can you update this? It'd also be great if you could take a look at the usage of gulp-util, it's probably not that hard to remove the dependency on it by using chalk, vinyl-fs etc directly. If not then I'll take a look after Christmas.

@rogierschouten
Copy link
Copy Markdown
Author

Ok I tried my best. I removed gulp-util, and updated all dependencies again for good measure.

Note that temporary typings were added for ansi-colors and plugin-error while my PRs on DefinitelyTyped are pending: DefinitelyTyped/DefinitelyTyped#22503 and DefinitelyTyped/DefinitelyTyped#22502

I can now run the tests - and they fail. It doesn't seem to be severe but I'm not confident in determining whether it is ok or not. @ivogabe could you have a look at them?

@phated
Copy link
Copy Markdown

phated commented Dec 26, 2017

@rogierschouten vinyl no longer allows the base property to be an empty string. It either has to be a string value (to set it) or a null/undefined value (to unset it). Empty string will throw with that message.

@rogierschouten
Copy link
Copy Markdown
Author

rogierschouten commented Dec 26, 2017

@phated I'm actually not getting that error on my machine ("works on my machine"(tm) ;) ). The code also seems to set base just fine. In my case the tests run and they give errors like this.

[21:54:11] Using gulpfile ~/Documents/github/gulp-typescript/gulpfile.js
[21:54:11] Starting 'clean'...
[21:54:11] Starting 'typecheck-dev'...
[21:54:11] Starting 'typecheck-2.3'...
[21:54:11] Starting 'clean-test'...
[21:54:11] Finished 'clean' after 27 ms
[21:54:11] Starting 'scripts'...
[21:54:11] Finished 'clean-test' after 102 ms
[21:54:12] Finished 'typecheck-2.3' after 1.59 s
[21:54:13] Finished 'typecheck-dev' after 2.57 s
[21:54:13] Starting 'typecheck'...
[21:54:13] Finished 'typecheck' after 25 μs
[21:54:14] Finished 'scripts' after 3.4 s
[21:54:14] Starting 'test-run'...
[21:54:31] Finished 'test-run' after 16 s
[21:54:31] Starting 'test'...

/home/rogier/Documents/github/gulp-typescript/test/output/basic/2.4/js/other-3.js 

    define(["require", "exports"], function (require, exports) {
        "use strict";
        exports.__esModule = true;
        var Hello = (function () {
        var Hello = /** @class */ (function () {
            function Hello() {
            }
            return Hello;
        }());
        exports.Hello = Hello;
    });
    
    //# sourceMappingURL=other-3.js.map
    
[21:54:31] 'test' errored after 44 ms
[21:54:31] Error in plugin 'gulp-diff'
Message:
    Files differ

But @phated @ivogabe to be honest I'm not willing to spend more of my vacation on this. I fixed two plugins of my own and sent 7 PRs to others, so I feel I've done my part. I hope one of you guys has some time left to finish this, and I hope this PR has been a good starting point. Sorry for leaving this hanging.

@phated
Copy link
Copy Markdown

phated commented Dec 26, 2017

@rogierschouten I appreciate you tackling all of that! Have a good rest of your vacation.

@ivogabe ivogabe mentioned this pull request Dec 27, 2017
@ivogabe ivogabe merged commit 58fdda5 into ivogabe:master Dec 27, 2017
@ivogabe
Copy link
Copy Markdown
Owner

ivogabe commented Dec 27, 2017

Thanks for your work @rogierschouten! I updated the test infrastructure to TypeScript 2.6 and added a package-lock.json and Travis is now passing. See #552.

@phated
Copy link
Copy Markdown

phated commented Dec 27, 2017

Awesome! Thanks @ivogabe for finishing this up 😸 and again thanks to @rogierschouten for getting it started!!!!

@demurgos
Copy link
Copy Markdown
Contributor

@ivogabe
Thanks for the update.
Could you publish the fixed version to npm?

@ivogabe
Copy link
Copy Markdown
Owner

ivogabe commented Jan 10, 2018

You can install gulp-typescript@4.0.0-alpha.1 now.

@simonua
Copy link
Copy Markdown

simonua commented Feb 2, 2018

Hi @ivogabe, do you have an idea of when this may be released as a non-alpha, please?

@ivogabe
Copy link
Copy Markdown
Owner

ivogabe commented Feb 8, 2018

Released it yesterday, I wanted to await the 2.7 release of TypeScript.

@simonua
Copy link
Copy Markdown

simonua commented Feb 8, 2018

Thank you very much. @ivogabe!

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 this pull request may close these issues.

5 participants