Fix TypeError for custom properties fallback in length-zero-no-unit

[shirotech]

e.g.

font: var(--Funnel-header-font, 600 16px/1.5 var(--font-header));

lineHeightNode is undefined, so we just need to check for it.

[jeddy3]

@shirotech Thank you for the pull request.

Can you add a test case for this, please?

You'll also need to run npm run format for the linting check to pass.

You'll find more information about running tests and formatting code in the contributing guide.

[shirotech]

Actually I found there is an option for ignore: ["custom-properties"] but it is not taking effect, the lint should skip if this option is found. I will need to look into fixing this properly and provide tests as well, thanks for looking at this. Having said that, if the option is not found it should also find another way to detect the lineHeightNode.

@jeddy3 what are your thoughts on quickly fixing it, for now, to at least not have any errors, and we can do a follow-up PR to address this properly for the "clean" way mentioned above?

[m-allanson]

When descending into functions, the nodes array will be scoped to the current function args. This is the array that nodeIndex refers to. See the postcss-value-parser docs for details.

e.g., to parse the value font: var(--foo, normal normal 400 16px/0 cursive);, the value parser will run two loops.

The outer loop operates on the following nodes:

// parsedValue.nodes contains this array:
[
      { type: 'word', sourceIndex: 0, value: 'font' },
      { type: 'div', sourceIndex: 4, value: ':', before: '', after: ' ' },
      {
        type: 'function',
        sourceIndex: 6,
        value: 'var',
        before: '',
        after: '',
        nodes: [
          [Object], [Object],
          [Object], [Object],
          [Object], [Object],
          [Object], [Object],
          [Object], [Object],
          [Object], [Object],
          [Object]
        ]
      }

The inner loop (equivalent to parsedValue.nodes[2].nodes) runs on the following values:

[
      { type: 'word', sourceIndex: 10, value: '--foo' },
      { type: 'div', sourceIndex: 15, value: ',', before: '', after: ' ' },
      { type: 'word', sourceIndex: 17, value: 'normal' },
      { type: 'space', sourceIndex: 23, value: ' ' },
      { type: 'word', sourceIndex: 24, value: 'normal' },
      { type: 'space', sourceIndex: 30, value: ' ' },
      { type: 'word', sourceIndex: 31, value: '400' },
      { type: 'space', sourceIndex: 34, value: ' ' },
      { type: 'word', sourceIndex: 35, value: '16px' },
      { type: 'div', sourceIndex: 39, value: '/', before: '', after: '' },
      { type: 'word', sourceIndex: 40, value: '0' },
      { type: 'space', sourceIndex: 41, value: ' ' },
      { type: 'word', sourceIndex: 42, value: 'cursive' }
    ]

Using nodes here means we can pick the right node with nodes[nodeIndex+1].

[m-allanson]

I've updated this and added a couple of tests. It should be ok for review now.

@shirotech the ignore: ["custom-properties"] option will only run on custom property declarations like:

a { --x: 0px; }

It doesn't handle var functions like:

font: var(--foo, normal normal 400 16px/0 cursive);

I'd recommend opening a separate issue to talk about the custom-properties option.

[shirotech]

Thanks @m-allanson anything else do we need to get this merged? Very keen on re-enabling this rule.

[jeddy3]

@m-allanson Thanks! LGTM now.

[mattxwang]

On my end, LGTM! We should open another issue discussing the custom properties option if it hasn't been done already.

[jeddy3]

Changelog:

• Fixed: length-zero-no-unit TypeError for custom properties fallback (#4860).

---

We should open another issue discussing the custom properties option if it hasn't been done already.

We can wait for a user to open one as I believe this pull request resolves the issue at hand.