svglint icon indicating copy to clipboard operation
svglint copied to clipboard

Output is truncated on MacOS (without --ci)

Open janeklb opened this issue 3 years ago • 8 comments

Update: see further down for details on bug (output truncated on macos)


Original message:

Attempting to set this linter up for my project but running into the following error

$ ./node_modules/.bin/svglint  --debug src/assets/icons/*.svg
-------------------------------------------------------- Log --------------------------------------------------------
(x) (node:33848) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
(Use `node --trace-warnings ...` to show where the warning was created)
(x) Failed to parse config: /Users/<redacted>/.svglintrc.js:1
export default {
^^^^^^

SyntaxError: Unexpected token 'export'
    at wrapSafe (internal/modules/cjs/loader.js:1001:16)
    at Module._compile (internal/modules/cjs/loader.js:1049:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:790:12)
    at ModuleWrap.<anonymous> (internal/modules/esm/translators.js:199:29)
    at ModuleJob.run (internal/modules/esm/module_job.js:183:25)
    at async Loader.import (internal/modules/esm/loader.js:178:24)
    at async file:///Users/<redacted>/node_modules/svglint/bin/cli.js:65:28

I've got a .svglintrc.js file with the following:

export default {
  rules: {
    attr: {
      width: true,
      height: true,
    }
  }
}

Using node 14.18.1 on MacOS 12.1

Am I using it wrong?

janeklb avatar Jan 26 '22 17:01 janeklb

Interestingly, if i try it with the following config

module.exports = {
  rules: {
    attr: {
      width: true,
      height: true
    }
  }
};

it just crashes without any output

$ ./node_modules/.bin/svglint  --debug src/assets/icons/*.svg
$ echo $?
1

But if i try it with this

module.exports = {
  rules: {
    attr: {
    }
  }
};

It runs well

$ ./node_modules/.bin/svglint src/assets/icons/*.svg
-------------------------------------------------- Summary --------------------------------------------------
✓ 108 valid files.
$ echo $?
0

janeklb avatar Jan 26 '22 17:01 janeklb

Regarding the original issue description

That SVGLint configuration is written using ESModule syntax (i.e. using export default {}), whereas your project expects CommonJS (i.e. using module.exports = {}), which are incompatible. As far as I know there's no easy/foolproof way to handle this programmatically, so I'd rather let users deal with using the correct syntax for their configuration.

That said, this could probably be better explained in the README (Pull Request is welcome 🙂).

Regarding the second comment

I'm not able to reproduce the crash-without-input based on just the configuration you provided (see below).

Are you able to share your project your setting this up with? If not, perhaps a minimum project that reproduces it? Also, could you share the OS you're trying this on.

The setup I tested with:

// testing/.svglintrc.js

module.exports = {
  rules: {
    attr: {
      width: true,
      height: true
    }
  }
};
// testing/a.svg

<svg role="img" viewBox="0 0 24 24">
    <g id="foo">
        <path d="bar"></path>
    </g>
    <g></g>
    <circle></circle>
</svg>
// testing/package.json

{
  "name": "testing",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "lint": "svglint --debug a.svg"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "svglint": "^2.1.0"
  }
}

The run npm install && npm run lint and I get

  At node <path> (2:8)
  x attr 2:8 Expected attribute 'height', didn't find it
               At node <path> (2:8)
  x attr 4:4 Expected attribute 'width', didn't find it
               At node <g> (4:4)
  x attr 4:4 Expected attribute 'height', didn't find it
               At node <g> (4:4)
  x attr 5:4 Expected attribute 'width', didn't find it
               At node <circle> (5:4)
  x attr 5:4 Expected attribute 'height', didn't find it
               At node <circle> (5:4)

------------------------------------------------------- Summary -------------------------------------------------------
x 1 invalid files.

ericcornelissen avatar Jan 27 '22 13:01 ericcornelissen

Thanks. I think i figured out the issue with the silent exit (with exit code 1) -- seems like it didn't like the input files i provided (i'm assuming one of them is "bad" in some way and causes svglint to crash -- will reproduce and raise another ticket for nicer failure handling).

In the meantime, how should I structure my config such that it checks for width and height on <svg> tags only? I'm seeing something about a "rule::selector" rule, but it's not clear how that should be used.

janeklb avatar Jan 27 '22 16:01 janeklb

It looks like the "silent failure" issue doesn't have to do with a single file, but rather with a large number of files (ie, lots of output, with failures) and not using --ci. So, i guess, something stemming from the GUI#update method.

I'm using MacOS 12.1 on iTerm2 + zsh

Note, when i run the testing files you posted above, i get truncated output - probably related to the same issue: image

janeklb avatar Jan 27 '22 16:01 janeklb

Maybe related to detecting terminal width.. if i stretch my terminal open wider i get: image

(ignore the <aws:org-sso> piece that's RPROMPT stuff)

janeklb avatar Jan 27 '22 16:01 janeklb

In the meantime, how should I structure my config such that it checks for width and height on <svg> tags only? I'm seeing something about a "rule::selector" rule, but it's not clear how that should be used.

The "rule::selector" rule is indeed what you want. You need to set it to a query selector of the elements for which the rule should hold, so in your case the following should work:

// testing/.svglintrc.js// testing/.svglintrc.js

module.exports = {
  rules: {
    attr: {
      width: true,
      height: true,
      "rule::selector": "svg"
    }
  }
};

It looks like the "silent failure" [(with exit code 1)] issue doesn't have to do with a single file, but rather with a large number of files (ie, lots of output, with failures) and not using --ci. So, i guess, something stemming from the GUI#update method.

I'm using MacOS 12.1 on iTerm2 + zsh

Note, when i run the testing files you posted above, i get truncated output - probably related to the same issue:

At this point I'm going to assume the "truncated output" and "silent failure" are related problems for simplicity, we'll find out if that's the case down the line :slightly_smiling_face:

I have now tested this on

  1. Ubuntu 20.04 with Zsh in default terminal application - no issue
  2. Windows 10 in Powershell in the new Windows terminal - no issue
  3. MacOS 12.1 with Zsh in the default terminal application - same issue!!

Maybe related to detecting terminal width.. if i stretch my terminal open wider i get:

I tested this as well, the terminal width doesn't influence whether or not the output is truncated for me, instead it looks something like:

-------------- Files --------------
x a.svg
  x attr 0:0 Expected attribute 'w
             idth', didn't find it
               At node <svg> (0:0)
  x attr 0:0 Expected attribute 'h
             eight', didn't find i
             t
               At node <svg> (0:0)

------------- Summary -------------
x 1 invalid files.

On the other hand the terminal height does seem to have an effect. With the above setup, if I make the terminal window tall enough there's no truncation. As I make the window shorter the output gets truncated from the top (which matches your findings so far).

Unfortunately, I only have limited access to a Mac so it might be hard for me to work on this. I'd encourage you to look into this yourself if you have time. In the meantime I guess you'd have to default to using the --ci option

ericcornelissen avatar Jan 28 '22 15:01 ericcornelissen

For what it's worth, the CLI rendering engine has given problems earlier and is likely due for a proper rewrite. I believe that the update logic I implemented back in the day (which is responsible for repeatedly rendering the GUI when --ci is not set) is poorly done - the --ci flag was added as a cheap workaround for an earlier issue with it.

While I can't comment on the issue reported here, it might be worthwhile to just redo the GUI implementation entirely.

birjj avatar Jan 28 '22 20:01 birjj

The "rule::selector" rule is indeed what you want. You need to set it to a query selector of the elements for which the rule should hold, so in your case the following should work:

Thanks - this worked. I swear I had tried this, but I suspect it had gotten mixed up with the truncated output issue and I didn't see the results one would expect. Cheers!

Running with --ci is not an issue for me at all 👍

janeklb avatar Jan 30 '22 10:01 janeklb