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

Fix TypeError for syntaxes that use Document in indentation #5767

Closed
43081j opened this issue Dec 7, 2021 · 3 comments · Fixed by #5771
Closed

Fix TypeError for syntaxes that use Document in indentation #5767

43081j opened this issue Dec 7, 2021 · 3 comments · Fixed by #5771
Labels
status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule

Comments

@43081j
Copy link
Contributor

43081j commented Dec 7, 2021

What steps are needed to reproduce the bug?

If you have a Root in a Document whose css begins on line 1, like so:

  .foo {
    color: hotpink;
  }

The indentation rule will throw, as it tries to access root.raws.beforeStart which doesn't exist.

What Stylelint configuration is needed to reproduce the bug?

{
  "rules": {
    "indentation": 2
  }
}

How did you run Stylelint?

CLI

Which version of Stylelint are you using?

14

What did you expect to happen?

Indentation rule would succeed.

What actually happened?

The rule throws:

TypeError: Cannot read properties of undefined (reading 'endsWith')

Does the bug relate to non-standard syntax?

No

Proposal to fix the bug

if (
(lastIndexOfNewline !== -1 ||
(isFirstChild && (!getDocument(parent) || parent.raws.beforeStart.endsWith('\n')))) &&
before.slice(lastIndexOfNewline + 1) !== expectedOpeningBraceIndentation
) {

Ultimately, we hit this condition. It is comprised of these:

Does node.raws.before contain a newline? - No, so we continue.

Is it the first child? - Yes.

Does it's parent have a document? - Yes.

This leads us to then attempting to execute parent.raws.beforeStart.endsWith('\n') which will fail if beforeStart doesn't exist.

We should null check parent.raws.beforeStart as it may never exist, or a plugin could have since removed it.

Btw I am aware of #5674 , so please let me know if you'll never fix this as I can fix it on my end instead (by populating beforeStart with an empty string at least).

43081j added a commit to 43081j/postcss-lit that referenced this issue Dec 7, 2021
We trim this so we don't trigger stylelint's rules which disallow empty
first lines. In JS sources, an empty first line makes sense.

We also have to set `beforeStart` explicitly on the root's raws until
stylelint fixes stylelint/stylelint#5767.
@ybiquitous
Copy link
Member

@43081j Thank you for creating this issue and using the template!

I cannot reproduce the error on this demo, so could you share more detailed reproduction, please?


We should null check parent.raws.beforeStart as it may never exist,

This seems needed to me.

@ybiquitous ybiquitous added status: needs reproducible example triage needs reproducible demo or repo type: bug a problem with a feature or rule labels Dec 8, 2021
@43081j
Copy link
Contributor Author

43081j commented Dec 8, 2021

this causes it:

import stylelint from 'stylelint';
import {stringify, Document, Input} from 'postcss';
import postcssParse from 'postcss/lib/parse';

const parse = (source, opts) => {
  const doc = new Document();
  const root = postcssParse(source, opts);
  root.parent = doc;
  root.document = doc;
  doc.nodes.push(root);
  doc.source = {
    input: new Input(source, opts),
    start: {line: 1, column: 1, offset: 0}
  };
  return doc;
};

(async () => {
  const code = `.foo {
    color: hotpink;
  }`;
  await stylelint.lint({
    customSyntax: {
      parse,
      stringify
    },
    code,
    codeFilename: 'foo.js',
    config: {
      rules: {
        indentation: 2
      }
    }
  });
})();

@ybiquitous ybiquitous added status: ready to implement is ready to be worked on by someone and removed status: needs reproducible example triage needs reproducible demo or repo labels Dec 9, 2021
@ybiquitous
Copy link
Member

@43081j Thank you for providing the reproduction! I understand the bug's behavior. 👍🏼

I think we should do the following:

I've labeled the issue as ready to implement. Please consider contributing if you have time.

There are steps on how to fix a bug in a rule in the Developer guide.

@jeddy3 jeddy3 changed the title Indentation rule assumes beforeStart is set for first child Fix TypeError for syntaxes that use Document in indentation Dec 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule
Development

Successfully merging a pull request may close this issue.

2 participants