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 with workaround a TypeError thrown for "html" #4797

Merged
merged 1 commit into from Jun 2, 2020
Merged

Conversation

jeddy3
Copy link
Member

@jeddy3 jeddy3 commented May 21, 2020

Which issue, if any, is this issue related to?

Closes #4793

Is there anything in the PR that needs further explanation?

It's not clear what's happening here, and a deeper investigation is needed. It appears the HTML syntax is somehow interfering with JavaScript files that contain the word "html".

We can workaround the issue by not lazy loading the HTML syntax ourselves and instead relying on postcss-syntax mechanism. We can do this because we are using the original postcss-html module, unlike with postcss-markdown and postcss-css-in-js where we are using our own forks.

I'll create a follow-up issue at some point to discuss what we do about postcss-syntax going forward. At the moment the syntax inferring is being handled by an unmaintained package.

Copy link
Member

@mattxwang mattxwang left a comment

Choose a reason for hiding this comment

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

Seems like a reasonable workaround for now, LGTM!

@m-allanson m-allanson merged commit 7ad3434 into master Jun 2, 2020
@m-allanson m-allanson deleted the issue-4793 branch June 2, 2020 15:09
@m-allanson
Copy link
Member

Updated changelog:

  • Fixed: workaround CSS-in-JS syntax throws a TypeError in the following conditions: 1. encounter a call or template expression named 'html' and 2. syntax loads via autoSyntax().

@m-allanson
Copy link
Member

I've created a reproduction for this issue here: https://github.com/m-allanson/stylelint-issue-4793

In case anyone wants to dig into this further. The bug is caused by using a call expression (such as html()) or a tagged template expression (such as html`foo`).

Here's how it's happening.

It's triggered when stylelint autoloads syntaxes. When autoloading syntaxes, stylelint adds the html syntax to its own options.

When running, the css-in-js syntax checks the global stylelint options for available syntaxes.

The css-in-js syntax sees that the options specify an html syntax, and so (incorrectly) assumes the html expression contains parseable styles.

It then runs into some unexpected behaviour, triggering a TypeError.

The error is triggered in the isStylePath function.

m-allanson added a commit that referenced this pull request Jun 3, 2020
# By Mike Allanson (6) and others
# Via GitHub
* master:
  Bump @types/lodash from 4.14.152 to 4.14.154 (#4817)
  Update CHANGELOG.md
  Fix with workaround a TypeError thrown for "html" (#4797)
  Update CHANGELOG.md
  Fix false positives for variables in font-family-no-missing-generic-family-keyword (#4806)
  Update CHANGELOG.md
  Add ignoreSelectors option to block-opening-brace-space-before (#4640)
  Update CHANGELOG.md
  Fix error message percentage/number precision for alpha-value-notation (#4802)
  Create new 'createPartialStylelintResult' module (#4815)
  Move function normalizeAllRuleSettings() out to a separate module (#4810)
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 Result.apply is not a function
4 participants