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 baseIndentLevel: 1 option and vue file in indentation #5657

Merged
merged 3 commits into from Nov 14, 2021
Merged

Fix TypeError for baseIndentLevel: 1 option and vue file in indentation #5657

merged 3 commits into from Nov 14, 2021

Conversation

wolfgangwalther
Copy link
Contributor

@wolfgangwalther wolfgangwalther commented Oct 26, 2021

This is a follow up to issue #5289, because PR #5297 did not replace syntax: 'html' and syntax: 'css-in-js' in rules tests. Those test-cases were disabled instead, and some of those were broken at the same time by accidentally removing some whitespace, where it was required for the tests.

The first and second commits replace syntax: ... everywhere and remove the skip: true in those cases. The third commit brings back the accidentally removed whitespace. After that, a few tests still fail:

  • the indentation and max-empty-lines rules were using raws.beforeStart and raws.afterEnd. Not sure where those came from (postcss-syntax maybe?), but it seems they should be replace with raws.codeBefore and raws.codeAfter. (fourth commit)
  • the declaration-empty-line-before rule seems to need another check for isRoot(parent) - at least that fixed the failing test... (fifth commit). There's still some issue with typing, but I don't really know typescript well, yet, so I'd need some guidance there.

I guess those changes should have been part of #5304, but were not noticed because of the skipped tests.


I came here, because when migrating to stylelint 14 in a vue project using postcss-html, I had the following error thrown:

TypeError: Cannot read properties of undefined (reading 'match')
...
at inferRootIndentLevel (node_modules/stylelint/lib/rules/indentation/index.js:682:25)

This only happens when setting baseIndentLevel to something other than auto.

Closes #5677

@jeddy3
Copy link
Member

jeddy3 commented Oct 26, 2021

@wolfgangwalther Thanks for the pull request.

The changes to codeBefore and codeAfter LGTM. They're from the official PostCSS 8 API.

postcss-html recently gained a maintainer and was updated for PostCSS 8, so we're fine to add that back in as a dev dependency and reenable those tests.

I don't believe postcss-css-in-js is actively maintained, nor updated to PostCSS 8 yet and relies on the unmaintained postcss-syntax. The package is going to be deprecated. If a leaner, library-specific and maintained library comes along that's designed for PostCSS 8, we can consider using that and reenabling the applicable tests. In the meantime, I don't think we should add postcss-css-in-js back as a dev dependency. Although, let's see if anyone else chimes in on this before we revert those parts of this pull request.

@jeddy3
Copy link
Member

jeddy3 commented Oct 26, 2021

It's probably neater (and easier to switch syntaxes) if we use strings for the customSyntax properties in testRule, e.g.:

{
  "customSyntax": "postcss-scss"
}

We can do that in another pull request though, so that this can focus on fixing issues with postcss-html and Stylelint 14.

@wolfgangwalther
Copy link
Contributor Author

In the meantime, I don't think we should add postcss-css-in-js back as a dev dependency. Although, let's see if anyone else chimes in on this before we revert those parts of this pull request.

I split that commit into two and can now easily drop the post-css-in-js part when this is confirmed.

@wolfgangwalther
Copy link
Contributor Author

I dropped the postcss-css-in-js changes (still have them on a different branch, if needed) and moved the change regarding the declaration-empty-line-before into #5689.

The remaining bits are just enabling postcss-html and fixing #5677. This should be GTM.

@jeddy3 jeddy3 changed the title Replace left over syntax option and fix broken tests Fix TypeError for baseIndentLevel: 1 option and vue file in indentation Nov 8, 2021
Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@jeddy3 jeddy3 mentioned this pull request Nov 8, 2021
6 tasks
@wolfgangwalther
Copy link
Contributor Author

(just a rebase on latest main)

Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wolfgangwalther Thanks. LGTM 👍🏼

@jeddy3 jeddy3 merged commit ef54485 into stylelint:main Nov 14, 2021
@jeddy3
Copy link
Member

jeddy3 commented Nov 14, 2021

Changelog entry:

  • Fixed: indentation TypeError for baseIndentLevel: 1 option for Vue files (#5657).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Fix TypeError for baseIndentLevel: 1 option and vue file in indentation
3 participants