Skip to content
This repository has been archived by the owner on Sep 16, 2022. It is now read-only.

Errors parsing CSS #348

Closed
zoechi opened this issue Apr 15, 2017 · 26 comments
Closed

Errors parsing CSS #348

zoechi opened this issue Apr 15, 2017 · 26 comments
Assignees

Comments

@zoechi
Copy link

zoechi commented Apr 15, 2017

Errors parsing CSS:
error on line 939, column 14: problem parsing function expected ), 
  color: rgb($color-accent);
             ^
error on line 939, column 14: expected }, but found $
  color: rgb($color-accent);
             ^
error on line 939, column 27: expected {, but found )
  color: rgb($color-accent);
                          ^
error on line 3073, column 25: problem parsing function expected ), 
  background-color: rgb($color-primary) !important; }
                        ^
error on line 3073, column 25: expected }, but found $
  background-color: rgb($color-primary) !important; }
                        ^
error on line 3073, column 39: expected {, but found )
  background-color: rgb($color-primary) !important; }
                                      ^
error on line 3073, column 41: parsing error expected }
  background-color: rgb($color-primary) !important; }
                                        ^^^^^^^^^^
error on line 3073, column 51: premature end of file unknown CSS
  background-color: rgb($color-primary) !important; }
                                                  ^
[DirectiveMetadataLinker on fhir_designer|lib/model/command/undo_service.ng_summary.json]:
Errors parsing CSS:
error on line 939, column 14: problem parsing function expected ), 
  color: rgb($color-accent);
             ^
error on line 939, column 14: expected }, but found $
  color: rgb($color-accent);
             ^
error on line 939, column 27: expected {, but found )
  color: rgb($color-accent);
                          ^
error on line 3073, column 25: problem parsing function expected ), 
  background-color: rgb($color-primary) !important; }
                        ^
error on line 3073, column 25: expected }, but found $
  background-color: rgb($color-primary) !important; }
                        ^
error on line 3073, column 39: expected {, but found )
  background-color: rgb($color-primary) !important; }
                                      ^
error on line 3073, column 41: parsing error expected }
  background-color: rgb($color-primary) !important; }
                                        ^^^^^^^^^^
error on line 3073, column 51: premature end of file unknown CSS
  background-color: rgb($color-primary) !important; }
                                                  ^
[StylesheetCompiler on bwu_ng_mdl|lib/src/template.css]:
Errors parsing CSS:
error on line 939, column 14: problem parsing function expected ), 
  color: rgb($color-accent);
             ^
error on line 939, column 14: expected }, but found $
  color: rgb($color-accent);
             ^
error on line 939, column 27: expected {, but found )
  color: rgb($color-accent);
                          ^
error on line 3073, column 25: problem parsing function expected ), 
  background-color: rgb($color-primary) !important; }
                        ^
error on line 3073, column 25: expected }, but found $
  background-color: rgb($color-primary) !important; }
                        ^
error on line 3073, column 39: expected {, but found )
  background-color: rgb($color-primary) !important; }
                                      ^
error on line 3073, column 41: parsing error expected }
  background-color: rgb($color-primary) !important; }
                                        ^^^^^^^^^^
error on line 3073, column 51: premature end of file unknown CSS
  background-color: rgb($color-primary) !important; }

Angular2: version: "3.0.0-beta+1"

@leonsenft
Copy link
Contributor

That's because $color-accent isn't valid CSS as far as I know. Are you using SASS/SCSS to generate your CSS? That looks like a SASS/SCSS variable that wasn't resolved.

Could you provide the source file(s) causing this error for reference please?

@leonsenft leonsenft self-assigned this Apr 15, 2017
@matanlurey
Copy link
Contributor

Ping @zoechi - I don't want to miss for 3.0.0 if an actual bug.

@zoechi
Copy link
Author

zoechi commented Apr 18, 2017

It's a bit hard to tell what source causes this error. It's the MDL CSS.
The error message would be more helpful if it contained the selector.
There is CSS like

.mdl-color--accent {
  background-color: unquote("rgb(#{$color-accent})") !important;
}

@zoechi
Copy link
Author

zoechi commented Apr 18, 2017

When I check the content of the file in the browser, there is no rgb($

@leonsenft
Copy link
Contributor

It won't show in the browser because the CSS shim won't output it. Are you using the latest version of MDL? I ran it through the shim recently while fixing the last issue you posted and I didn't see this error, but I'll look again.

@leonsenft
Copy link
Contributor

I just checked the latest version of MDL and found this segment.

.mdl-color--accent {
  background-color: rgb(255, 64, 129) !important;
}

This corresponds to the original SCSS it was compiled from, which is exactly the snippet you provided. Are you perhaps using the SCSS directly in your project?

@zoechi
Copy link
Author

zoechi commented Apr 18, 2017

If I load http://localhost:63342/myproj/web/designer.css
I also get

.mdl-color--accent {
  background-color: rgb(255,64,129) !important; }

but I still get the error in the transformer

@leonsenft
Copy link
Contributor

It seems like somehow the SCSS is being fed into the transformer. Is your project hosted somewhere so I could take a look? That link is to localhost so I can't see your CSS.

@zoechi
Copy link
Author

zoechi commented Apr 18, 2017

Sorry, the localhost link wasn't supposed to work, I just wanted to show how I got the content (so that the browser or shims won't modify it)
It's a big project. I have to break it down to create a smaller repo.
I won't be able to do that before Thursday.

@leonsenft
Copy link
Contributor

Alright thanks. Sorry you're running into this issue. Like I said, I suspect SCSS is being fed into the transformer in place of CSS somewhere. Are you using SCSS in your project and compiling it to CSS?

@zoechi
Copy link
Author

zoechi commented Apr 18, 2017

@leonsenft
Copy link
Contributor

Is there a chance it's outputting SCSS that is being fed into Angular? Do you have copy-sources set to true? Can you run that transformer in isolation and check if the output contains this above snippet still?

@zoechi
Copy link
Author

zoechi commented Apr 20, 2017

@leonsenft Where does copy-sources come from (where can it be set/removed)?

@leonsenft
Copy link
Contributor

@zoechi
Copy link
Author

zoechi commented Apr 20, 2017

I see :D. No, I'm not using it. I don't apply any options at all to the sass transformer.

@leonsenft
Copy link
Contributor

Sorry, without the actual project to look at I'm not sure how I can help you any further. As I said before this issue isn't a bug with the CSS parser, but rather a bug or misconfiguration with tooling that SCSS is ending up in the style compiler directly.

@zoechi
Copy link
Author

zoechi commented Apr 20, 2017

@leonsenft I still plan to provide more information. Just wanted to get clarification on other question raised during the discussion.

@leonsenft
Copy link
Contributor

Okay great, I still absolutely want to help you resolve this issue.

@zoechi
Copy link
Author

zoechi commented Apr 26, 2017

I was able to investigate more.

The code complained about

  color: rgb($color-accent);
             ^

seems to come back from the SASS transformer as shown in the error message, when the variables (like $color-accent) are not defined.

What's weird is, that template.scss isn't referenced anywhere, it just exists in a project that is a pub dependency of the application project.

@leonsenft
Copy link
Contributor

So because the variable isn't defined, it's never replaced and remains in the CSS?

The second issue you've described is because the transformer processes all CSS files in the project (and dependencies) #352. It's possible to exclude files from being processed in your pubspec.yaml, but this has to be done in the project containing the file itself, not from a dependent project.

I'm going to close this issue because it's correctly reporting invalid CSS. There's discussion though about revising the approach taken with new CSS shimming to be less restrictive.

@zoechi
Copy link
Author

zoechi commented Apr 26, 2017

Perhaps it shouldn't break the build on invalid CSS for files that don't concern it (not referenced in @Component(...).
Also if I exclude the file in the containing project, how am I supposed to use this file when I actually want to?
To me this still seems broken.

@leonsenft
Copy link
Contributor

You're right, that's a valid concern which is covered by issue #352.

@zoechi
Copy link
Author

zoechi commented Apr 27, 2017

@ok, great. Somehow missed the link.

@zoechi
Copy link
Author

zoechi commented Apr 27, 2017

After being able to get rid of this error, I was able (or the new Angular version did - not sure) to fix some other problem that caused serious troubles since months (see for example #351).

I didn't expect that above error really breaks the Angular build, but now it looks like it did.
Because after excluding the SASS file that caused above error output,
seems to also have fully fixed #352 that haunted me since about 10/2016

Now I have the suspicion that the broken CSS caused the problems all along and the updated CssLib just made it visible.

When I remove the exclude: lib/src/template.scss line, I for example get

[web] GET packages/fhir_webui/src/webui_generated/form/edit_mode_service.template.dart => Could not find asset fhir_webui|lib/src/webui_generated/form/edit_mode_service.template.dart.

and errors like this happened all the time. Here it complains about the template of a service!!!
and it complained about a different service every time, sometimes also other stuff.
This was about to drive me cracy.
Some random changes in the code and reloading the page (without restarting pub serve) made the application work fine (most of the time).

To me this now looks like changing the code and reloading made parts of the Angular transformer re-run which wasn't run previously because of the CSS problem (maybe because it threw an exception),
while the part that processes the CSS wasn't run again, because no CSS did change (only some code file) and therefore this time didn't cause an exception.

It's one side to print error messages about stuff that is not correct, but another to break the build, and I think invalid CSS should not break the build.
It's fine of course that it reports issues, but I think it should otherwise allow the transformer to continue normally.

I think there is still some ugly issue hidden that can lead to really bad user experience,
like the error message about edit_mode_service above because of broken CSS.

If this explanation is too confusing, please let me know and I try to improve.
I'm not sure if this problem driving me cracy succeeded (to some degree),
and I am no longer able to express myself properly ;-)

@leonsenft
Copy link
Contributor

The broken CSS shouldn't have been responsible before we introduced the parser from csslib. Before everything was regex based, so it wouldn't even know if there was invalid CSS. What happens now, is we parse the CSS, then shim it by transforming the CSS AST.

The issue is if csslib itself runs into an error parsing. It doesn't have very good error recovery, so rather than failing to shim a single selector, it has rather unpredictable results, but generally gives up on the rest of the file. We thought this should be a build error since it's unlikely if you had a CSS error that your component would look anything like you intended. But having CSS errors in files you're not using break the build is not very desirable behavior, so maybe we should revert the CSS errors to warnings.

The issues are further exacerbated by csslib only accepting known at-rules with specific grammars, rather than performing more generalized parsing.

Overall I'm not very happy with the amount of disruption this change has caused, as it was just intended to fix some bugs with the shim. I'm going to work on making the parser much more lenient in what it accepts which should eliminate this problem and the others like it.

@zoechi
Copy link
Author

zoechi commented Apr 27, 2017

The broken CSS shouldn't have been responsible before we introduced the parser from csslib.

but when I remove the exclusion or template.scss I get the same error that I got repeatedly the last months (at the beginning it was a different error but otherwise similar erratic behavior)

We thought this should be a build error

Build errors are usually very annoying. I don't want to be forced to have every character in the whole application perfect to be able to look at the result.
Usually during refactoring or otherwise in the middle of some development I want to be able to check a single component or part of the application and don't care at all about whether in some other parts styling is broken (because of a recent change or similar).
This is one of the strengths of Dart that an application runs fine as long as broken code is not executed.
I understand that this can't fully work with Angulars code generation, but I don't see a need to drop this feature entirely because of CSS, where also the browsers play along and are as forgiving as it gets.
I think this is extremely important for a quick edit/reload cycle.
(Just wondering how this will work with DDC :-/)
I assume that this also will be quite surprising behavior for most web developers.

Overall I'm not very happy with the amount of disruption

In the end it seems to have fixed (or at least revealed some insight about) an issue that was extremely frustrating.

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

No branches or pull requests

3 participants