-
-
Notifications
You must be signed in to change notification settings - Fork 17
feat: safely parse mangled CSS with safeParser
#225
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis PR introduces a new optional Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/beasties/test/parse.test.ts (1)
78-101: Good basic test coverage.The test successfully verifies that malformed CSS doesn't throw an error when
safeParseris enabled. This provides essential smoke test coverage for the feature.You could optionally enhance the test to verify that the CSS rule is actually processed (e.g., checking that the style tag contains some CSS), though the current basic coverage is acceptable:
// Verify the HTML was processed without throwing expect(result).toContain('Test') expect(result).toContain('data-beasties-container') + // Optionally verify CSS was processed + expect(result).toContain('<style>') + expect(result).toContain('.test') })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/beasties-webpack-plugin/test/fixtures/fs-access/dist/style.cssis excluded by!**/dist/**packages/beasties/package.jsonis excluded by!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (6)
packages/beasties/README.md(1 hunks)packages/beasties/src/css.ts(1 hunks)packages/beasties/src/index.d.ts(1 hunks)packages/beasties/src/index.ts(1 hunks)packages/beasties/src/types.ts(1 hunks)packages/beasties/test/parse.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/beasties/src/index.ts (1)
packages/beasties/src/css.ts (1)
parseStylesheet(30-35)
packages/beasties/test/parse.test.ts (1)
packages/beasties/src/index.ts (1)
Beasties(34-707)
🔇 Additional comments (6)
packages/beasties/README.md (1)
142-142: Documentation looks good.The safeParser option is well-documented. The description accurately explains its purpose for fault-tolerant CSS parsing.
packages/beasties/src/index.d.ts (1)
66-66: LGTM!The type definition is correct and properly positioned in the Options interface.
packages/beasties/src/types.ts (1)
99-102: LGTM!The type definition with JSDoc is clear and correctly documents the default behaviour.
packages/beasties/src/css.ts (2)
30-34: LGTM!The implementation correctly uses optional chaining and maintains backward compatibility by making the options parameter optional.
22-22: Repository is missing critical package.json files.The repository structure is incomplete: no
package.jsonexists at root level, nor in any of the package directories (packages/beasties/,packages/beasties-webpack-plugin/,packages/vite-plugin-beasties/). This prevents verification of whetherpostcss-safe-parseris listed as a dependency. The css.ts import statement is present, but dependency resolution cannot be confirmed without package.json files. Please verify that the repository is in the correct state and that all package.json files are present before this review comment can be resolved.packages/beasties/src/index.ts (1)
429-430: LGTM!The safeParser option is correctly propagated to both the main AST and the inverse AST parsing calls, ensuring consistent parsing behaviour throughout the processing pipeline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks great! any reason not to enable by default? 🤔
it would also be great to add a test 🙏
safeParser
Adds a
safeParseroption to use https://github.com/postcss/postcss-safe-parser when parsing stylesheets. This allows parsing CSS that may contain some errors.