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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

parcel 2.4.0 breaks some CSS queries #7854

Closed
aldrakos opened this issue Mar 23, 2022 · 29 comments
Closed

parcel 2.4.0 breaks some CSS queries #7854

aldrakos opened this issue Mar 23, 2022 · 29 comments
Labels

Comments

@aldrakos
Copy link

馃悰 bug report

After updating to parcel 2.4.0, @parcel/transformer-css errors for some more edge case css rules.
E.g.
@media screen and (min-width: 0\0) and (min-resolution: +72dpi) {
errors during build and
@media screen\0 {
is transformed in a way that it's ignored.

馃 Expected Behavior

CSS tranform works as before and correctly allows specific @media rules.

馃槸 Current Behavior

Css transform throws an error in some cases, or ignores some other rules.
E.g.
@parcel/transformer-css: Unexpected token Colon
for
@media screen and (min-width: 0\0) and (min-resolution: +72dpi) {

馃拋 Possible Solution

Revert to previous Parcel version.

@devongovett
Copy link
Member

What is this syntax: 0\0? I don't think that's valid CSS...

@aldrakos
Copy link
Author

These are both a little weird CSS "hacks" for IE detection and rules. The first one is actually part of the datepicker library I used (flatpickr).

The second CSS rule is also an IE detection rule and was used by myself to show a broweser upgrade warning for unsupported browsers.

@devongovett
Copy link
Member

Seems like what the @supports rule is for. I'm not sure we should support invalid CSS hacks like this.

@aldrakos
Copy link
Author

As mentioned it's part of an external library used (flatpickr) that uses that for styling. I'll see about replacing or overriding that then. Thanks for the feedback!

@leicht-io
Copy link

I have the same error. It is reproducible when adding the following:

@-moz-document url-prefix("") {
    .ui-side-panel-scrollable-content {
        overflow-y: auto
    }
}

It gives the following error:

@parcel/transformer-css: Unexpected token QuotedString("")

devongovett added a commit to parcel-bundler/lightningcss that referenced this issue Mar 24, 2022
@katzmo
Copy link

katzmo commented Mar 24, 2022

Also ran into this issue with 3rd party css using @media screen and (min-width: 0\0) (an issue there was closed as "not a bug" with a reference to http://browserbu.gs/css-hacks/media-min-width-0-backslash-0/). I think whatever works for CSS should not cause any error in @parcel/transformer-css.

@devongovett
Copy link
Member

devongovett commented Mar 24, 2022

Sorry, but that's not valid CSS syntax. All modern browsers will completely ignore that rule. I don't think Parcel should be expected to implement Internet Explorer's bugs. The @supports rule is specifically designed for detecting supported features and providing fallbacks.

I think the code linked in that issue could be rewritten just using @supports.

background-image: url("data:image/png;...");

@supports (background-image: url("data:image/svg+xml;utf8,<svg></svg>")) {
  background-image: url("data:image/svg+xml;utf8...");
}

@devongovett
Copy link
Member

Maybe we can see about sending some PRs to these libraries to replace these old hacks...

@katzmo
Copy link

katzmo commented Mar 25, 2022

Sorry, but that's not valid CSS syntax. All modern browsers will completely ignore that rule. I don't think Parcel should be expected to implement Internet Explorer's bugs.

I agree it would be better to not use this old hack any more but it鈥檚 a bit harsh that the css-transformer refuses to build any CSS that uses it. I鈥檓 not asking to implement IE bugs, I would just expect the css-transformer to do what any modern browser does: ignore it.

@devongovett
Copy link
Member

I think most of the time it is much more useful for a build tool to show syntax errors than to ignore them. Otherwise you might miss one and accidentally ship broken code, or have to spend a long time debugging to figure out why your css isn't being applied as you expect.

@katzmo
Copy link

katzmo commented Mar 25, 2022

For finding syntax errors I鈥檒l use a linter, e.g. stylelint or prettier. Usually, I only want to know about syntax errors in my own code though, not some other node packages.

@eldang
Copy link

eldang commented Mar 31, 2022

I'm running into the same problem due to a dependency's use of a different old-IE hack:

馃毃 Build failed.

@parcel/transformer-css: Unexpected token Delim('*')

  /Users/eldan/Documents/github/terragis/ol_base_viewer/node_modules/datatables.net-dt/css/jquery.dataTables.css:138:3
    137 |   cursor: pointer;
  > 138 |   *cursor: hand;
  >     |   ^
    139 |   background-repeat: no-repeat;
    140 |   background-position: center right;

While I would prefer to have https://github.com/DataTables/Dist-DataTables-DataTables stop using it, I don't have control over their code so for now I'm working around by not updating Parcel. I would very much prefer for Parcel's bundler to either ignore this like all modern browsers do, or warn without refusing to build.

@devongovett
Copy link
Member

Maybe you could use https://github.com/s10wen/stylehacks to remove these. It's a postcss plugin, so you just need a .postcssrc like this:

{
  "plugins": {
    "stylehacks": {}
  }
}

I tested it with the above code and it seems to work.

@eldang
Copy link

eldang commented Mar 31, 2022

This looks like it should work, but I'm missing something about how to use it. I installed stylehacks as per the instructions there, made a .postcssrc as you suggest, and parcel build gives me the same error. I also tried configuring it with postcss.config.js as suggested at https://github.com/postcss/postcss#parcel . Are there additional steps I should be doing to get postcss to run? (or possibly just to make it run before @parcel/transformer-css?)

I've also tried adding:

  "transformers": {
  	"*.css": ["@parcel/transformer-postcss", "..."]
  }

to .parcelrc, with the "..." at the start or end or omitted, all to no avail. I'm sorry, I feel like I'm probably missing something that should be obvious, but it would be so much easier if there were just a way to stop Parcel from trying to enforce validation on external dependencies.

@mischnic
Copy link
Member

That should be working. You don't need a parcelrc, the postcss transformer is part of the default config:

"*.{css,pcss}": ["@parcel/transformer-postcss", "@parcel/transformer-css"],

@eldang
Copy link

eldang commented Apr 11, 2022

Empirically, it either isn't working, or the addition of stylehacks doesn't achieve the intended goal. I did try quite a few permutations of the config, including not touching .parcelrc.

@eldang
Copy link

eldang commented Apr 11, 2022

Is it possible that transformer-css gets the first pass over the code, and if so is there any way we can control that with the config? I have also tried:

  "transformers": {
  	"*.css": ["@parcel/transformer-postcss"]
  }

in .parcelrc hoping it would take transformer-css out of the loop entirely, but still got @parcel/transformer-css: Unexpected token Delim('*')

@devongovett
Copy link
Member

PostCSS config doesn't apply to code in node_modules. Maybe that's the issue for you?

@eldang
Copy link

eldang commented Apr 11, 2022

Ah! That sounds likely, because this is specifically an issue caused by a Node dependency, which is why I can't just fix the CSS in my own project.

@eldang
Copy link

eldang commented May 3, 2022

Unfortunately, this is continuing to make it impossible for me to keep my project up to date with Parcel's releases. Which is a shame because you're doing really great work with the bundler in general. But I'm stuck with a dependency that I don't have the power to make compliant.

@devongovett
Copy link
Member

You always have the power to take ownership of your dependencies. For example, you could fork it or use something like patch-package. Or you could build a custom Parcel plugin to do it like the one someone posted here: parcel-bundler/lightningcss#39 (comment).

I'm open to an opt-in mode where things are less strict, but it's not as simple as just not erroring, we'd need to implement error recovery. My free time is limited though and it's not my top priority. Luckily, there are lots of ways to work around things like this.

@eldang
Copy link

eldang commented May 4, 2022

Thank you for these suggestions. Forking this particular dependency would add a great deal of work because of the tooling that it in turn needs to build it, but the other two options look realistic. I will try one or both in the coming week or so and report back in case that ends up being helpful to anyone else having similar problems.

@eldang
Copy link

eldang commented May 11, 2022

Thanks again! patch-package is a good solution for my needs, was easy to set up, and also has nice support for turning ad-hoc packages into PRs for the source repository.

@devongovett
Copy link
Member

In parcel-bundler/lightningcss@69cbcca I implemented a new errorRecovery option which will ignore invalid rules and declarations rather than erroring (just like a browser would). They are omitted from the output, and a warning is emitted instead. Hopefully this allows people to work around issues in libraries more easily. Working on getting it released soon.

@hacdias
Copy link

hacdias commented Jul 26, 2022

@devongovett any ETA for the release?

@devongovett
Copy link
Member

Parcel 2.7.0 supports the errorRecovery option.

@vishaldodiya
Copy link

vishaldodiya commented Mar 13, 2023

@devongovett Is there a doc for errorRecovery option?

I did try to use in package.json but it does not seem to work.

  "@parcel/transformer-css": {
    "errorRecovery": false
  }

@docBliny
Copy link

@devongovett Is there a doc for errorRecovery option?

I did try to use in package.json but it does not seem to work.

@ vishaldodiya Try setting errorRecovery to true instead of false. Worked for me (though I got additional warnings as well compared to not using it).

@NealEhardt
Copy link

NealEhardt commented Jun 9, 2023

I'm using Yarn Workspaces and one of my dependencies has buggy CSS. I had to configure errorRecovery in the top-level package.json, even though my Parcel devDependency is only defined in one of the child workspaces.

{
  "name": "my-root-package",
  "workspaces": ["client", "server"],
  "scripts": { ... },
  "devDependencies": {},
  "dependencies": {},
  "@parcel/transformer-css": {
    "errorRecovery": true
  }
}

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

No branches or pull requests

10 participants