segmentio / myth

A CSS preprocessor that acts like a polyfill for future versions of the spec.
4.32k stars 131 forks source link

Better error message when @import fail #62

Closed MoOx closed 10 years ago

MoOx commented 10 years ago

Currently I just got

[gulp] Plumber found unhandled error: [gulp] TypeError in plugin 'gulp-myth': Cannot read property '0' of null

Which is clearly not easy to understand where the fuck I break something.

The stack trace help me to understand it was in rework-inline (I where using rework-npm that doesn't need .css ext) but I think it can be nice to catch the error somewhere to warn the issue came from @import. Do you think it's possible ?

Anyway thanks for Myth !

ianstormtaylor commented 10 years ago

Ah yeah, the error needs to be nicely formatted from the rework-import (there was a standard system for Rework mentioned somewhere) and then Myth should display it more nicely at that point

tdhsmith commented 10 years ago

Yeah there's a bit of a bug with rework-inline... it does have error checking here:

Import.prototype._check = function (name) {
    var file = findFile(name, { path: this.path, global: false })[0];

    if (!file) {
        throw new Error('failed to find ' + name);
    }

    return file;
};

But with the way find-file works, unfound files return null, so file is being set to null[0] and we never get to the proper error checking. I don't know about the "standard" Rework system @ianstormtaylor mentions, but I'll look into it.

tdhsmith commented 10 years ago

Oh I'm guessing you mean rework's error(msg) function (line 259 here). Gives line number and source file and such... I'll look into it, but I'm gonna submit a trivial fix first.

MoOx commented 10 years ago

I just make a quick PR as a workaround for now

https://github.com/kevva/rework-inline/pull/4

tdhsmith commented 10 years ago

Sweet thanks. Exactly what I was gonna do. :+1:

raribeiro commented 10 years ago

In version 1.0.3 I am having the following problem in the output: Warning: Cannot read property '0' of null Use --force to continue.

The problem happens when I use Google Fonts: @import url(http://fonts.googleapis.com/css?family=Open+Sans:400,700,600);

tdhsmith commented 10 years ago

Well there seem to be a couple intersecting problems here.

  1. The issue @MoOx fixed above, which is just a bit of broken error checking. That's what actually generates the "Cannot read property '0'" warning.
  2. parse-import doesn't seem to accept unquoted url() imports, which should be valid. (cf. issue 1)
  3. Most importantly though, rework-inline doesn't seem to know how to inline non-local files. I'm new to myth and rework, so I don't know the best way to approach this, but one of the alternatives, rework-import, sidesteps the issue by simply ignoring urls that start with "http".

As for actually avoiding the issue right now, I'd probably just comment out that line while running myth and manually add it back in the output?

MoOx commented 10 years ago

FYI, rework-inline has been merged into rework-import (better name). Better error reporting are in master of this last repo & will be published soon (probably with a rework-inline depreciation notice). This will also handle local (relative) files & ignore http url. I'll make a PR right after the release.

MoOx commented 10 years ago

Closed by 129b389b647f647b105bdc0d9921c6c5a1599b32