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

Output is truncated on MacOS (without --ci) #56

Open
janeklb opened this issue Jan 26, 2022 · 8 comments
Open

Output is truncated on MacOS (without --ci) #56

janeklb opened this issue Jan 26, 2022 · 8 comments

Comments

@janeklb
Copy link

janeklb commented Jan 26, 2022

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
Copy link
Author

janeklb commented Jan 26, 2022

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

@ericcornelissen
Copy link
Contributor

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.

@janeklb
Copy link
Author

janeklb commented Jan 27, 2022

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
Copy link
Author

janeklb commented Jan 27, 2022

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
Copy link
Author

janeklb commented Jan 27, 2022

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)

@ericcornelissen
Copy link
Contributor

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 🙂

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 ericcornelissen changed the title Failed to parse config Output is truncated on MacOS (without --ci) Jan 28, 2022
@birjj
Copy link
Contributor

birjj commented Jan 28, 2022

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.

@janeklb
Copy link
Author

janeklb commented Jan 30, 2022

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 👍

siimveskilt pushed a commit to e-gov/cvi that referenced this issue Jan 23, 2023
Merge in SUN/veera-components from refactor-EBS-459-storybook-structure to master

Squashed commit of the following:

commit e973ef75cb5aff152a0e31993e6165c74642b009
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 20:35:47 2023 +0200

    format(storybook): minor formatting

commit 2ad02af4a5cf80b13b8f88e7766cd2f801f58ae6
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 20:33:42 2023 +0200

    refactor(icons): removed hardcoded color from user icon (to be added in an app itself)

commit e8fd48a7c4bdc43b61c5f2b73a7c5ab060c00d81
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 20:30:36 2023 +0200

    refactor(icons): removed hardcoded color from sort icon (to be added in an app itself)

commit 5708ff702196b79c2f9a4334693be698b9bfaa47
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 20:28:03 2023 +0200

    refactor(icons): removed hardcoded color from happy-face and sad-face icons (to be added in an app itself)

commit 2fab7fa6c12e953a932fae1b7fcd48bf6656c499
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 20:26:09 2023 +0200

    refactor(icons): removed an unnecessary fill attribute from info icon

commit 7c6fb5c9f39442eef160081b71e9dd699d5f3fc1
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 20:21:34 2023 +0200

    refactor(icons): simplified path in error-outline icon

commit 0d57fda39179c49aa46097c7f1324d8a05e37300
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 20:15:49 2023 +0200

    refactor(icons): removed an unused attribute from email icon

commit 1fb3ce3f6d52a3cb323d7dee24a701bdf0f1ffaf
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 20:11:36 2023 +0200

    refactor(icons): removed unnecessary group tags from all SVG icons

commit c98e53f9d71145884b52836016e3f6e4e58e1d77
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 20:06:30 2023 +0200

    feat(icons): mark as a linting error presence of a g tag inside SVG markup with only one child

    see simple-icons/svglint#56

commit 909737f29f983014167267986fdd1cc4a9cfd979
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 20:03:14 2023 +0200

    feat(icons): do not truncate errors list when linting icons

    see simple-icons/svglint#56

commit eaa4bbf72bfacf7ea6cddf03fc55d1f5ef977461
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 20:01:59 2023 +0200

    refactor(icons): simplified path in check-circle-outline icon

commit b100f0e59272b738d915e52276a4c6b45b0fa848
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 20:01:14 2023 +0200

    refactor(icons): removed hardcoded color and opacity from arrow-up-alt icon (to be added in an app itself)

commit b3093c5d80c88d16ca75a5ee2ff450efea5cadef
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 19:56:20 2023 +0200

    feat(icons): added more rules to SVGLint config

commit 75364d48869d08799d1f52eb9c87719838887aea
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 19:55:38 2023 +0200

    refactor(icons): removed hardcoded color from edit icon (to be added in an app itself)

commit 8e3556503caf41ff135f610f73efd3bf47d0df01
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 19:55:11 2023 +0200

    refactor(icons): removed hardcoded color and opacity from arrow-down-alt icon (to be added in an app itself)

commit 841649248b51c3bcdc3943b85d147ddeea78090d
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 19:10:07 2023 +0200

    refactor(icons): removed hardcoded color from arrow-up icon (to be added in an app itself)

commit 446e6bf9503512c044438fbe2187de20599a1a69
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 18:52:27 2023 +0200

    refactor(icons): removed hardcoded color from arrow-down icon (to be added in an app itself)

commit f69e285a06847c6faec28ad7d5a5dfd73161e451
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 18:51:53 2023 +0200

    refactor(icons): removed hardcoded color from action icon (to be added in an app itself)

commit f2c3e8b87bb558d13a45a086923d13029b36e6c6
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 18:43:52 2023 +0200

    feat(icons): added an SVG linter and lint:svg command

commit 3229f0ad8d6ddf9456684f11eba83337615ff808
Author: Aleksandr Beliaev <[email protected]>
Date:   Sun Jan 15 18:12:54 2023 +0200

    feat(storybook): expand height of All icons story when viewed in Docs tab

... and 5 more commits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants