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

Use NodeErrorOptions directly from the postcss types #133

Merged
merged 1 commit into from
Apr 9, 2021
Merged

Use NodeErrorOptions directly from the postcss types #133

merged 1 commit into from
Apr 9, 2021

Conversation

realityking
Copy link
Contributor

@realityking realityking commented Mar 30, 2021

A quick follow-up to #132. postcss/postcss#1549 was merged very quickly so now NodeErrorOptions can be referenced without a deep import.

This includes the required upgrade to PostCSS 2.8.9 that includes the above fix.

@realityking
Copy link
Contributor Author

Tests are failing all over. This must be related to the PostCSS bump.

@shellscape
Copy link
Owner

Looks like snapshots out of date due to the version bump. Some additions and removals from the shape of return objects. You'll have to go over each one individually, but from the few I looked at it looks like it's safe to update the snapshots.

@realityking
Copy link
Contributor Author

Will do. Do you use some tool to generate the snapshots or is that a manual process?

@realityking
Copy link
Contributor Author

@shellscape Can you give me a hint how the snapshots are created? Thanks!

@shellscape
Copy link
Owner

This requires at least postcss 2.8.9
@realityking
Copy link
Contributor Author

Thanks! That worked :)

I had a look through the snapshots and I don't think I can review them. The changes have accumulated over multiple PostCSS versions and I don't know enough about the inner workings of PostCSS.

What I could do, it submit a number of smaller PRs that would have smaller, more isolated changes to the snapshots.

What do you think?

@shellscape
Copy link
Owner

I think you're letting the size of a diff overwhelm you a bit. The differences in the snapshots are almost exclusively around internal Input methods fromOffset and isComplete, which have either been removed by PostCSS in the latest version or replaced with a Symbol. Neither of these should have an impact on users, but we can release a major version just to be sure no one is negatively impacted.

@realityking
Copy link
Contributor Author

You're not wrong about the size 😄 But it's compounded by the fact I'm not that familiar with the interface of PostCSS.

I'd be happy to break it into smaller upgrades that I'd be more comfortable reviewing. Let me know if you wanna do that.

@shellscape
Copy link
Owner

I've taken another look over this and I think we're good to go. Dealing with sick kids this week so it may be a delayed release.
thanks!

@shellscape shellscape merged commit 0357c82 into shellscape:master Apr 9, 2021
@realityking realityking deleted the types-fix branch April 9, 2021 15:29
@realityking
Copy link
Contributor Author

Thanks for taking a look & all the best for your kids 🙂

@joscha
Copy link

joscha commented Apr 27, 2021

@shellscape any chance we could get a new release with this change? Cheers!

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.

3 participants