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

Capture the CssSyntaxError object and throw error. #6

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

aga5tya
Copy link

@aga5tya aga5tya commented Jan 22, 2017

Fixes #5

@giuseppeg
Copy link
Owner

@aga5tya thank you! Can you add a test?

@aga5tya
Copy link
Author

aga5tya commented Jan 23, 2017

@giuseppeg,, do i have to include a failure test case ? like a scenario expecting an error on the wrong styles. I'm new to unit testing stuff,, apologies in case its a trivial question.

@giuseppeg
Copy link
Owner

@aga5tya hey no worries!

like a scenario expecting an error on the wrong styles

correct I would add a new fixture to tests/fixtures with some css error e.g. a missing semicolon:

export default () => (
  <div>
    <style jsx>{`
        p {
           color: red
           width: 100%
        }
    `}</style>
  </div>
)

And then the test in test/index.js use t.throws https://github.com/avajs/ava#throwsfunctionpromise-error-message

@aga5tya
Copy link
Author

aga5tya commented Feb 1, 2017

@giuseppeg , please review and guide. Updated the PR.

@rickyrauch
Copy link

+1 Great

@giuseppeg
Copy link
Owner

giuseppeg commented Feb 3, 2017

@aga5tya sorry I've been busy. The PR looks good, thank you!

While we are on it I was thinking that maybe we can print out some info about the error source too, what do you think?

I saw that the postcss error object has a showSourceCode method that is really nice!

Also since the PostCSS processor is not loading the css from a file we get something like:

CssSyntaxError: <css input>:1:32: Missed semicolon

Would you be interested in fixing this too? <css input> should be replaced with the component path and 1:32 with the locs in the component. Basically getCss should also return start and end – something like this

}
`}</style>
</div>
)
Copy link
Owner

Choose a reason for hiding this comment

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

needs an empty line at the end of the file

@giuseppeg
Copy link
Owner

@aga5tya want me to finish this branch?

@rickyrauch
Copy link

can't wait for this

@aga5tya
Copy link
Author

aga5tya commented Mar 23, 2017

@giuseppeg, please do if you have time, i have been busy with lot of other things, won't have time until a month.

@rickyrauch
Copy link

we have styled-jsx-postcss working on scaleapi.com 💯

@metasean
Copy link

Just curious, when will this get pulled into the current release?

@giuseppeg
Copy link
Owner

I want to see if we can ship plugins support to styled-jsx (there is an open PR for it) and in case convert this repo to a plugin for styled-jsx instead of doing parallel development.

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

Successfully merging this pull request may close these issues.

4 participants