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

Arguments to path.join must be string. #118

Closed
jzvelc opened this issue Jul 8, 2014 · 11 comments
Closed

Arguments to path.join must be string. #118

jzvelc opened this issue Jul 8, 2014 · 11 comments
Assignees

Comments

@jzvelc
Copy link

jzvelc commented Jul 8, 2014

I am unable to implement grunt-ts in my grunt due to following error when running task:
Compiling...
Warning: Arguments to path.join must be strings Use --force to continue.

I have tried almost everything without success.

@basarat
Copy link
Member

basarat commented Jul 8, 2014

@jzvelc I suspect it is because of grunt-contrib-watch

@jzvelc
Copy link
Author

jzvelc commented Jul 8, 2014

Well it is not, I tried running ts task alone and I get same error. I have tried all different configurations without success (even specifying src and outDir alone).

@basarat
Copy link
Member

basarat commented Jul 8, 2014

Well it is not, I tried running ts task alone and I get same error.

cool. Thanks for that, I'll investigate

@basarat
Copy link
Member

basarat commented Jul 9, 2014

@jzvelc have you found an easy way to reproduce this?

@basarat
Copy link
Member

basarat commented Jul 9, 2014

This snippet at the top of your ts.js file would be nice :

var oldJoin = path.join;
path.join = function () {
    _.forEach(arguments, function (argument) {
        if (typeof argument !== 'string') {
            try {
                console.log('why no string!'.red);
                console.log('BAD path.join called', arguments);
                throw new Error('why no string!');
            } catch (ex) {
                console.log(ex.stack);
            }
        }
    });

    return oldJoin.apply(path, arguments);
};

So the start of you ts.js looks like :

/// <reference path="../defs/tsd.d.ts"/>
/// <reference path="./modules/interfaces.d.ts"/>
/*
* grunt-ts
* Licensed under the MIT license.
*/
// Typescript imports
var _ = require('underscore');
var path = require('path');
var fs = require('fs');

var oldJoin = path.join;
path.join = function () {
    _.forEach(arguments, function (argument) {
        if (typeof argument !== 'string') {
            try {
                console.log('why no string!'.red);
                console.log('BAD path.join called', arguments);
                throw new Error('why no string!');
            } catch (ex) {
                console.log(ex.stack);
            }
        }
    });

    return oldJoin.apply(path, arguments);
};

@basarat
Copy link
Member

basarat commented Jul 9, 2014

I have a stacktrace

image

It seems targetName is undefined here : https://github.com/grunt-ts/grunt-ts/blob/master/tasks/modules/compile.ts#L60 i.e. grunt.task.current.target is undefined

Seems like a grunt bug : gruntjs/grunt#994 (comment)

@basarat
Copy link
Member

basarat commented Jul 9, 2014

lesson learnt, grunt.task.current.target is not reliable. The reason is that it might not point to grunt-ts at the time of code completion is completed e.g. in this case it was pointing to grunt-contrib-watch which obviously does not have the target member :
image

This itself is probably because for some reason done of the grunt-ts task is called before we try to clear the cache which is why grunt moves back to watch task.

Possible Solutions:

a.) Store and pass the targetname around : simple to do and guaranteed to work
b.) track down why grunt moved on without completing us (grunt-ts). I suspect its because of the way watch interruption is setup and it may not be in our control.

I'll go with a.) since it is bound to work.

@basarat
Copy link
Member

basarat commented Jul 9, 2014

I'll assign this issue to myself if I work on it. PRs welcome

@basarat
Copy link
Member

basarat commented Jul 9, 2014

worth mentioning based on the analysis that it should have no bad side effect. Its simply failing when grunt has moved on which means that whatever is trying to complete shouldn't even be there.

@basarat basarat self-assigned this Jul 21, 2014
basarat added a commit that referenced this issue Jul 21, 2014
- Needed because of #118 being nasty even in build
@basarat basarat mentioned this issue Jul 21, 2014
@basarat
Copy link
Member

basarat commented Jul 21, 2014

fixed in v1.11.5

@basarat
Copy link
Member

basarat commented Jul 21, 2014

Store and pass the targetname around : simple to do and guaranteed to work

This is what I did because grunt.task.current.target seemed to be always undefined in my latest npm install for some reason.

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

No branches or pull requests

2 participants