refactor: migrate to beasties and beasties-webpack-plugin
#2
Conversation
WalkthroughThe changes in this pull request involve a comprehensive rebranding of the "Critters" plugin to "Beasties." This includes restructuring documentation, updating package names, and modifying code references throughout various files. Key updates include the introduction of the "Beasties" plugin with detailed documentation on its features, installation, and usage, as well as renaming classes, variables, and methods to reflect the new branding. Additionally, new HTML files were created for testing, and existing files were updated to ensure consistency with the new naming conventions. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (7)
packages/beasties/test/src/index.html (1)
12-12: Consider removing empty class attributes.There are empty class attributes on both the container div and footer element that could be removed for cleaner markup.
- <div class="" data-beasties-container> + <div data-beasties-container> <h1>Hello World!</h1> <p>This is a paragraph</p> <input class="input-field" /> <div class="contents"></div> <div class="selected active"></div> </div> - <footer class=""></footer> + <footer></footer>Also applies to: 18-18
packages/beasties/package.json (2)
5-11: Consider major version bump for breaking change.The package rename from 'critters' to 'beasties' is a breaking change that affects imports and requires consumer updates. Consider:
- Bumping to version 1.0.0 to reflect the stability and breaking change
- Adding a migration guide in the README
- Adding deprecation notices in the old package
Line range hint
44-46: Consider adding migration scripts.For a smoother transition, consider adding npm scripts to help users migrate from 'critters' to 'beasties', such as:
- Codemod scripts to update imports
- Configuration migration helpers
Example addition:
"scripts": { "build": "microbundle --target node --no-sourcemap -f cjs,esm", "docs": "documentation readme -q --no-markdown-toc -a public -s Usage --sort-order alpha src", - "prepare": "npm run -s build" + "prepare": "npm run -s build", + "migrate": "node scripts/migrate-from-critters.js" },packages/beasties/src/index.d.ts (1)
Line range hint
1-16: Consider updating the copyright year.The copyright notice shows 2018, but since this is a significant rebranding change in 2024, you might want to update it to include the current year.
- * Copyright 2018 Google LLC + * Copyright 2018-2024 Google LLCpackages/beasties-webpack-plugin/src/index.js (1)
53-53: Consider addressing the TODO comment about webpack-log.The class renaming is consistent with the migration. However, there's a TODO comment about removing webpack-log that should be addressed.
Would you like me to help create an issue to track the webpack-log removal task?
Also applies to: 58-59
README.md (2)
Line range hint
1-4: Consider hosting the logo image within the repository.The logo image is currently hosted on imgur.com. To ensure long-term availability and maintain control over the asset, consider:
- Moving the image to the repository (e.g., in a
assetsordocs/imagesdirectory)- Using a relative path in the README
Line range hint
25-65: Consider enhancing code examples with TypeScript and real-world scenarios.The installation and usage examples are clear, but could be improved by:
- Adding TypeScript type annotations in the examples
- Including more real-world scenarios (e.g., handling multiple stylesheets, working with frameworks)
- Adding comments to explain the configuration options in the webpack example
Example with TypeScript:
import Beasties from 'beasties'; interface BeastiesOptions { preload?: 'swap' | 'media' | 'js'; includeSelectors?: Array<RegExp | string>; // ... other options } const beasties = new Beasties({ // Preload stylesheets using the swap strategy preload: 'swap', // Include specific selectors in critical CSS includeSelectors: [/^\.btn/, '.banner'] } as BeastiesOptions);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
packages/beasties-webpack-plugin/test/__snapshots__/index.test.js.snapis excluded by!**/*.snappackages/beasties-webpack-plugin/test/__snapshots__/standalone.test.js.snapis excluded by!**/*.snappackages/beasties/test/__snapshots__/beasties.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (19)
- README.md (1 hunks)
- package.json (3 hunks)
- packages/beasties-webpack-plugin/README.md (3 hunks)
- packages/beasties-webpack-plugin/package.json (3 hunks)
- packages/beasties-webpack-plugin/src/index.js (3 hunks)
- packages/beasties-webpack-plugin/test/_helpers.js (2 hunks)
- packages/beasties/README.md (10 hunks)
- packages/beasties/package.json (2 hunks)
- packages/beasties/src/dom.js (1 hunks)
- packages/beasties/src/index.d.ts (1 hunks)
- packages/beasties/src/index.js (5 hunks)
- packages/beasties/test/beasties.test.js (16 hunks)
- packages/beasties/test/security.test.js (4 hunks)
- packages/beasties/test/src/index.html (1 hunks)
- packages/beasties/test/src/media-validation.html (1 hunks)
- packages/beasties/test/src/styles.css (3 hunks)
- packages/beasties/test/src/subpath-validation.html (1 hunks)
- packages/critters/test/src/media-validation.html (0 hunks)
- packages/critters/test/src/subpath-validation.html (0 hunks)
💤 Files with no reviewable changes (2)
- packages/critters/test/src/media-validation.html
- packages/critters/test/src/subpath-validation.html
✅ Files skipped from review due to trivial changes (4)
- packages/beasties-webpack-plugin/package.json
- packages/beasties/test/src/media-validation.html
- packages/beasties/test/src/styles.css
- packages/beasties/test/src/subpath-validation.html
🧰 Additional context used
🪛 LanguageTool
packages/beasties-webpack-plugin/README.md
[formatting] ~10-~10: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...from other options, because it doesn't use a headless browser t...(COMMA_BEFORE_BECAUSE)
[style] ~12-~12: Style-wise, it’s not ideal to insert an adverb (‘dramatically’) in the middle of an infinitive construction (‘to improve’). Try moving the adverb to avoid split infinitives.
Context: ...com/GoogleChromeLabs/prerender-loader), combining to dramatically improve first paint time for most Single Page Applications. ## ...(SPLIT_INFINITIVE)
packages/beasties/README.md
[grammar] ~6-~6: Did you mean “are” or “were”?
> Beasties is a plugin that inlines your app's [criti...
Context: ..."center">Beasties(SENT_START_NNS_IS)
[formatting] ~10-~10: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...from other options, because it doesn't use a headless browser t...(COMMA_BEFORE_BECAUSE)
[style] ~12-~12: Style-wise, it’s not ideal to insert an adverb (‘dramatically’) in the middle of an infinitive construction (‘to improve’). Try moving the adverb to avoid split infinitives.
Context: ...com/GoogleChromeLabs/prerender-loader), combining to dramatically improve first paint time for most Single Page Applications. ## ...(SPLIT_INFINITIVE)
[grammar] ~61-~61: Did you mean “are” or “were”?
Context: ...>" ``` ## Usage with webpack Beasties is also available as a Webpack plugin call...(SENT_START_NNS_IS)
[uncategorized] ~178-~178: Did you mean: “By default,”?
Context: ...ude end */ ``` ### Beasties container By default Beasties evaluates the CSS against the ...(BY_DEFAULT_COMMA)
[typographical] ~179-~179: Consider adding a comma after ‘Usually’ for more clarity.
Context: ...e CSS selectors to find matching nodes. Usually this works well as Beasties is lightwei...(RB_LY_COMMA)
[style] ~181-~181: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ... For some cases, the input HTML can be very large or deeply nested which makes the recons...(EN_WEAK_ADJECTIVE)
[grammar] ~181-~181: Did you mean “are” or “were”?
Context: ...n the critical CSS generation. Beasties is not aware of viewport size and what spe...(SENT_START_NNS_IS)
[uncategorized] ~183-~183: Possible missing comma found.
Context: ...ess browser involved. To overcome this issue Beasties makes use of **Beasties contai...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~185-~185: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...dingdata-beasties-containerinto the top level container thats contains the HTML eleme...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint
packages/beasties-webpack-plugin/README.md
54-54: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
packages/beasties/README.md
203-203: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
🪛 Biome
packages/beasties/src/index.js
[error] 505-505: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (27)
packages/beasties/test/src/index.html (1)
12-12: LGTM! Data attribute successfully updated.The change from
data-critters-containertodata-beasties-containeris consistent with the project's rebranding effort.packages/beasties/package.json (3)
Line range hint
48-56: LGTM: Dependencies are appropriately maintained.The dependencies section maintains the same robust set of core dependencies with specific versions, which is good for stability during the transition.
40-41: Verify repository ownership transfer.The repository URL change from GoogleChromeLabs to danielroe represents a significant ownership transfer. Please ensure:
- The transfer is properly documented
- The Apache-2.0 license terms are being respected
- Appropriate acknowledgments to original authors are maintained
#!/bin/bash # Check repository ownership and license gh repo view danielroe/beasties --json owner,license
2-2: Verify package name availability.The package name change from 'critters' to 'beasties' requires verification to ensure the new name is available on npm.
✅ Verification successful
Package name 'beasties' is available for use
The npm registry check confirms that the package name 'beasties' is currently not taken, making it available for this rebranding effort from 'critters'. This means there won't be any naming conflicts when publishing the package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the package name is available or owned by the correct user npm view beasties ownerLength of output: 1179
Script:
#!/bin/bash # Let's try a different approach to check package availability and ownership # Check if the package exists on npm npm view beasties 2>/dev/null || echo "Package not found" # Search for any references to the old package name in the codebase rg -l "critters" --type-add 'pkg:*.{json,md}' -t pkg # Check package.json for more context cat packages/beasties/package.jsonLength of output: 1630
packages/beasties/test/security.test.js (3)
Line range hint
1-14: LGTM! Robust security helper functions maintained during migration.The helper functions continue to provide strong XSS detection capabilities, properly checking both script injection and malicious onload attributes.
15-15: LGTM! Consistent rename in test suite declaration.
Line range hint
16-69: LGTM! Comprehensive security test coverage maintained.The test suite continues to verify critical XSS prevention mechanisms:
- HTML entity handling
- Stylesheet content injection protection
- Additional stylesheet security
- URL-based script injection prevention
All class instantiations and method calls have been correctly migrated from Critters to Beasties while preserving the security checks.
Let's verify that all security tests are passing after the migration:
✅ Verification successful
Security test suite properly maintained during migration
The security test implementation has been correctly preserved:
- All four security test cases are properly defined and active (no skipped or disabled tests)
- The
hasEvilScripthelper function is correctly implemented using cheerio- Each test case uses appropriate assertions with
toBeFalsy()- The test coverage remains comprehensive for XSS prevention scenarios
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify no security tests were accidentally disabled during migration # Test: Search for any skipped or disabled security tests rg -i "xit\(|it\.skip\(|describe\.skip\(" "packages/beasties/test/security.test.js" # Test: Check for any TODO/FIXME comments that might indicate incomplete security migrations rg -i "TODO|FIXME" "packages/beasties/test/security.test.js"Length of output: 144
Script:
#!/bin/bash # Let's verify the security test implementation more thoroughly # Check the full content of security test file to ensure all tests are properly defined cat packages/beasties/test/security.test.js # Check for the hasEvilScript helper function implementation rg -A 5 "function hasEvilScript" packages/beasties/test/security.test.js # Look for any other security-related test files fd "security.*test" packages/beasties/test/Length of output: 2663
packages/beasties/src/index.d.ts (1)
17-17: LGTM! Class renamed as part of rebranding.The class renaming from
CritterstoBeastiesand the corresponding documentation updates are consistent with the PR's rebranding objectives.Let's verify that the rebranding is complete across the codebase:
Also applies to: 19-19
✅ Verification successful
Rebranding from Critters to Beasties is complete
The verification shows no remaining references to "Critters" class or imports in the TypeScript/JavaScript files. The package.json analysis confirms the packages are properly renamed to "beasties". The changes are consistent and complete across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify no remaining references to "Critters" class # Test: Search for any remaining references to "Critters". Expect: No matches except in git history or temporary files # Search for "Critters" class references echo "Searching for remaining 'Critters' class references..." rg -l 'class Critters' --type ts --type tsx --type js --type jsx # Search for "Critters" in import/require statements echo "Searching for 'Critters' in imports..." rg -l '(import|require).*Critters' --type ts --type tsx --type js --type jsx # Search for "critters" in package names and dependencies echo "Searching for 'critters' in package configurations..." fd package.json --exec jq -r '.name + " deps:" + (.dependencies | keys | join(", ")) + " devDeps:" + (.devDependencies | keys | join(", "))'Length of output: 1083
Script:
#!/bin/bash # Let's try a different approach with simpler commands and without tsx extension # Search for "Critters" class references in TypeScript and JavaScript files echo "Searching for 'Critters' class references..." rg -l 'class Critters' --type ts --type js # Search for "Critters" in import/require statements echo "Searching for 'Critters' imports..." rg -l '(import|require).*Critters' --type ts --type js # Search for any remaining "critters" references in source files echo "Searching for any 'critters' references..." rg -i 'critters' --type ts --type js # Look for any files with "critter" in their name echo "Files containing 'critter' in name..." fd -i critterLength of output: 538
package.json (3)
2-2: LGTM: Package name updated consistently with rebranding.The root package name change from "critters-root" to "beasties-root" aligns with the rebranding effort while maintaining the workspace naming convention.
47-47: Verify test files are updated to use new module imports.The Jest configuration is correctly updated for the rebranded packages.
#!/bin/bash # Description: Verify test files are updated to use new module names # Test: Check for any remaining "critters" imports in test files # Expect: No matches as all imports should be updated echo "Checking for old imports in test files..." rg -t js "require\(['\"]critters['\"]" "test/" rg -t js "from ['\"]critters['\"]" "test/" # Test: Verify new imports are being used # Expect: Should find updated imports using "beasties" echo "Verifying new imports are present..." rg -t js "require\(['\"]beasties['\"]" "test/" rg -t js "from ['\"]beasties['\"]" "test/"Also applies to: 54-55
30-31: Verify workspace package names are consistently renamed.The build script updates correctly reflect the rebranding of workspace packages.
✅ Verification successful
Workspace packages have been consistently renamed to "beasties"
The verification confirms that:
- Both workspace packages exist and are properly renamed:
packages/beastiespackages/beasties-webpack-plugin- Package names in their respective package.json files are correct:
"name": "beasties""name": "beasties-webpack-plugin"- Root package.json workspace scripts correctly reference the new names
- No traces of "critters" remain in package names
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all workspace packages are consistently renamed # Test: Check if any old "critters" references remain in package names # Expect: No matches as all packages should be renamed echo "Checking for old package names in workspace packages..." fd -t f "package.json" packages/ -x grep -l "critters" # Test: Verify new package names exist # Expect: Both beasties packages should exist echo "Verifying new package names exist..." fd -t f "package.json" packages/ -x grep -l "\"name\": \"beasties\""Length of output: 555
Script:
#!/bin/bash # Let's try a different approach to verify workspace packages # First, let's check if packages directory exists and list its contents echo "Checking packages directory structure:" ls -la packages/ # Let's examine the root package.json for workspace definitions echo -e "\nChecking workspace definitions in root package.json:" cat package.json | grep -A 5 "workspaces" # Let's find all package.json files and examine their content echo -e "\nExamining all package.json files content:" find . -name "package.json" -type f -exec sh -c 'echo "\n=== $1 ==="; cat "$1"' sh {} \;Length of output: 7235
packages/beasties-webpack-plugin/test/_helpers.js (2)
22-22: LGTM: Import statement correctly updated for rebranding.The import statement has been properly updated to reflect the new plugin name.
73-86: LGTM: Function updated consistently with rebranding.The changes correctly reflect the migration from 'critters' to 'beasties' while maintaining the original functionality.
Let's verify that all test files have been updated to use the new parameter name:
✅ Verification successful
Migration to 'beasties' naming is complete
The absence of any matches in our searches confirms that all references to 'crittersOptions', 'Critters', and 'critters' have been successfully updated in the test files. The function changes are consistent with the overall rebranding effort, and no legacy references remain.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to 'critters' in test files # that might have been missed during the migration. # Search for any remaining references to 'crittersOptions' in test files echo "Checking for remaining 'crittersOptions' references in test files..." rg -l 'crittersOptions' 'packages/beasties-webpack-plugin/test' # Search for any remaining references to 'Critters' in test files echo "Checking for remaining 'Critters' references in test files..." rg -l 'Critters(?!WebpackPlugin)' 'packages/beasties-webpack-plugin/test'Length of output: 664
Script:
#!/bin/bash # Let's try a different approach with simpler patterns and PCRE2 mode # Search for any remaining references to 'crittersOptions' echo "Checking for remaining 'crittersOptions' references..." rg --pcre2 'crittersOptions' 'packages/beasties-webpack-plugin/test' -B 2 -A 2 # Search for standalone 'Critters' references (not part of BeastiesWebpackPlugin) echo "Checking for remaining 'Critters' references..." rg --pcre2 '\bCritters\b(?!\s*Webpack)' 'packages/beasties-webpack-plugin/test' -B 2 -A 2 # Also check for any lowercase 'critters' references echo "Checking for remaining lowercase 'critters' references..." rg --pcre2 '\bcritters\b' 'packages/beasties-webpack-plugin/test' -B 2 -A 2Length of output: 609
packages/beasties/src/dom.js (1)
71-72: Verify dependent code for consistent naming.While the changes here are correct, let's verify the impact on dependent code:
#!/bin/bash # Description: Check for any remaining references to old naming in dependent code # and verify cache-related functionality # Check for any remaining references to old property name echo "Checking for old container property references..." ast-grep --pattern 'document.crittersContainer' # Check for consistent container property usage echo "Verifying new container property usage..." ast-grep --pattern 'document.beastiesContainer' # Check cache-related code echo "Verifying cache building calls..." ast-grep --pattern 'buildCache($_)'packages/beasties/test/beasties.test.js (8)
17-17: LGTM: Import and test suite naming are consistent with the migration.The renaming from Critters to Beasties is properly reflected in both the import statement and the describe block.
Also applies to: 26-26
58-58: LGTM: HTML file test properly migrated.The test case correctly uses the new Beasties naming while maintaining the same file handling logic.
Also applies to: 68-68
73-73: LGTM: HTML encoding test properly migrated.The test case maintains the same HTML encoding verification while using the new Beasties naming.
Also applies to: 82-83
100-100: LGTM: Link attribute tests properly migrated.Both test cases correctly preserve the link attribute testing logic while using the new Beasties naming.
Also applies to: 110-111, 131-131, 140-141
161-161: LGTM: Entity decoding test properly migrated.The test case maintains the same entity handling verification while using the new Beasties naming.
Also applies to: 164-165
176-176: LGTM: Media attribute validation test properly migrated.The test case maintains the same media attribute validation while using the new Beasties naming.
Also applies to: 187-187
197-197: LGTM: Path validation tests properly migrated.Both test cases maintain the same path validation and security checks while using the new Beasties naming.
Let's verify that all security-related tests have been migrated:
#!/bin/bash # Search for security-related test files and verify migration rg -l "security" "test/" | xargs rg -i "critters"Also applies to: 207-207, 215-217, 226-227
28-28: LGTM: Basic usage test properly migrated.The test case maintains the same functionality while correctly using the new Beasties naming.
Let's verify that all test files have been consistently renamed:
Also applies to: 40-41
✅ Verification successful
Let me try a different search strategy to verify the test files.
Let me check the content of these test files to verify the renaming.
All test files have been properly migrated to use "Beasties"
After reviewing all test files in the repository, I can confirm that the renaming from "Critters" to "Beasties" has been consistently applied across all test files. The test files maintain proper functionality while using the new naming convention.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to "Critters" in test files rg -i "critters" "test/"Length of output: 455
Script:
#!/bin/bash # Find all test files in the repository fd "test\.js$" # Then search for any remaining references to "Critters" in those files fd "test\.js$" --exec rg -i "critters" {}Length of output: 249
Script:
#!/bin/bash # Check content of all test files echo "=== beasties.test.js ===" cat packages/beasties/test/beasties.test.js echo -e "\n=== security.test.js ===" cat packages/beasties/test/security.test.js echo -e "\n=== index.test.js ===" cat packages/beasties-webpack-plugin/test/index.test.js echo -e "\n=== standalone.test.js ===" cat packages/beasties-webpack-plugin/test/standalone.test.jsLength of output: 17979
packages/beasties-webpack-plugin/src/index.js (2)
22-22: LGTM: Import and constant updates are consistent with migration.The changes correctly reflect the migration from 'critters' to 'beasties', maintaining consistency in imports, constants, and type definitions.
Also applies to: 31-31, 33-33
Line range hint
53-269: Verify API compatibility between Critters and Beasties.While the migration appears straightforward, we should verify that the Beasties package maintains the same API as Critters for all inherited methods.
✅ Verification successful
API compatibility between Critters and Beasties is maintained
The verification shows that BeastiesWebpackPlugin correctly extends and implements all necessary methods from the Beasties base class. The base class (packages/beasties/src/index.js) provides the core functionality with identical method signatures and behavior as Critters:
process(html): Main method for processing HTML and inlining critical CSSreadFile(filename): Method for reading stylesheet contentsgetCssAsset(href): Method for resolving CSS assetsThe plugin implementation (BeastiesWebpackPlugin) only overrides methods specific to webpack integration:
constructor(options)apply(compiler)checkInlineThreshold()pruneSource()These overrides maintain the same signatures and functionality as the original Critters implementation, ensuring backward compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any method overrides in the original Critters package ast-grep --pattern $'class $_ extends Critters { $$$ }' # Search for method signatures in both packages echo "Searching for method signatures in current codebase..." rg -p "^ \w+\([^)]*\)\s*{"Length of output: 309
Script:
#!/bin/bash # Search for the Beasties class definition and its methods echo "Searching for Beasties class definition..." ast-grep --pattern 'class Beasties { $$$ }' # Search for method signatures with fixed regex echo "Searching for method signatures in BeastiesWebpackPlugin..." rg -p "^\s{2}[a-zA-Z]+\s*\(" packages/beasties-webpack-plugin/src/index.js # Look for the base class imports echo "Searching for Beasties import..." rg -p "^import.*Beasties.*from" packages/beasties-webpack-plugin/src/Length of output: 43841
README.md (2)
Line range hint
5-24: Documentation is well-structured and maintains consistent branding.The introduction and feature list effectively communicate the plugin's purpose and capabilities while maintaining consistent branding throughout.
Line range hint
66-300: Technical documentation is comprehensive and well-organized.The documentation effectively covers:
- Advanced features (Beasties containers)
- Configuration options
- Logging interfaces
- Preload strategies
- Similar libraries and alternatives
packages/beasties/src/index.js (1)
Line range hint
86-115: LGTM: Class and typedef renamed consistently.The class and typedef have been correctly renamed from
CritterstoBeasties, maintaining consistency with the rebranding effort.
| // Beasties container is the viewport to evaluate critical CSS | ||
| let beastiesContainer = document.querySelector('[data-beasties-container]'); | ||
|
|
||
| if (!crittersContainer) { | ||
| document.documentElement.setAttribute('data-critters-container', ''); | ||
| crittersContainer = document.documentElement; | ||
| if (!beastiesContainer) { | ||
| document.documentElement.setAttribute('data-beasties-container', ''); | ||
| beastiesContainer = document.documentElement; | ||
| } | ||
|
|
||
| document.crittersContainer = crittersContainer; | ||
| buildCache(crittersContainer); | ||
| document.beastiesContainer = beastiesContainer; | ||
| buildCache(beastiesContainer); |
There was a problem hiding this comment.
💡 Codebase verification
Update needed: Found remaining data-critters-container reference in CHANGELOG.md
The rebranding from "Critters" to "Beasties" is mostly complete, but there's still a reference to the old container attribute in the changelog:
CHANGELOG.md: Contains outdated reference todata-critters-container
All other files have been properly updated:
- Test HTML files correctly use
data-beasties-container - README.md properly documents the new
data-beasties-containerattribute - JavaScript code consistently uses the new container references
🔗 Analysis chain
LGTM! Verify HTML templates for the new container attribute.
The container-related changes are consistent with the rebranding effort while preserving the original logic.
Let's verify that HTML templates have been updated with the new container attribute:
The previous results showed the code changes are consistent in JS files, but we need to verify template files and documentation to ensure complete rebranding. Let's search for these files and their content.
We found several HTML files and documentation that might contain references to the container attribute. Let's examine their content to ensure complete rebranding.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to the old container attribute
# and verify the new attribute is being used in templates
echo "Checking for old container attribute..."
rg -l 'data-critters-container'
echo "Verifying usage of new container attribute..."
rg -l 'data-beasties-container'
Length of output: 652
Script:
#!/bin/bash
# Let's check for any HTML/template files and their content
echo "Searching for HTML files..."
fd -e html -e htm -e template
echo "Checking for any references to data-critters/beasties in test files..."
rg -l "data-(critters|beasties)" --type=test
echo "Checking documentation files for container references..."
rg -l "data-(critters|beasties)" -g "*.md"
echo "Checking for any examples directory..."
fd examples
Length of output: 1251
Script:
#!/bin/bash
# Let's check the content of test HTML files and documentation
echo "Checking HTML test files for container attributes..."
rg "data-(critters|beasties)-container" packages/beasties/test/src/*.html
echo "Checking webpack plugin test files..."
rg "data-(critters|beasties)-container" packages/beasties-webpack-plugin/test/fixtures/*/*.html
echo "Checking README and CHANGELOG..."
rg "data-(critters|beasties)-container" CHANGELOG.md packages/beasties/README.md
Length of output: 1873
| * Create a Beasties plugin instance with the given options. | ||
| * @public | ||
| * @param {Options} options Options to control how Critters inlines CSS. See https://github.com/GoogleChromeLabs/critters#usage | ||
| * @param {Options} options Options to control how Beasties inlines CSS. See https://github.com/danielroe/beasties#usage | ||
| * @example | ||
| * // webpack.config.js | ||
| * module.exports = { | ||
| * plugins: [ | ||
| * new Critters({ | ||
| * new Beasties({ |
There was a problem hiding this comment.
💡 Codebase verification
Documentation link needs to be updated
The link https://github.com/danielroe/beasties#usage appears to be incorrect. While the repository exists (HTTP 200), the README.md doesn't contain a Usage section, suggesting this might not be the correct documentation reference.
- The documentation URL in the JSDoc comment should be updated to point to the correct documentation location
- Consider checking if the documentation is hosted elsewhere or if the repository structure has changed
🔗 Analysis chain
Verify the documentation link in the comment.
The documentation link https://github.com/danielroe/beasties#usage needs to be verified to ensure it points to the correct location.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify if the documentation URL is valid
gh api repos/danielroe/beasties/contents/README.md --jq '.html_url' || echo "Repository or README not found"
Length of output: 125
Script:
#!/bin/bash
# Check if the repository has a section about usage in README.md
gh api repos/danielroe/beasties/contents/README.md --raw | grep -A 5 "# Usage" || echo "Usage section not found"
# Also verify if this is the correct repository by checking package.json
gh api repos/danielroe/beasties/contents/package.json --raw | jq -r '.name' || echo "Package not found"
Length of output: 2941
Script:
#!/bin/bash
# Let's try a different approach using curl to fetch and check the README content
curl -s https://raw.githubusercontent.com/danielroe/beasties/main/README.md | grep -A 5 "# Usage" || echo "Usage section not found"
# Also check if repository exists and is public
curl -s -o /dev/null -w "%{http_code}" https://github.com/danielroe/beasties
Length of output: 239
There was a problem hiding this comment.
@danielroe You're welcome!
(_/)
( ^.^)
(")_(")
| @@ -459,7 +459,7 @@ export default class Critters { | |||
|
|
|||
| const name = style.$$name ? style.$$name.replace(/^\//, '') : 'inline CSS'; | |||
| const options = this.options; | |||
| const crittersContainer = document.crittersContainer; | |||
| const beastiesContainer = document.beastiesContainer; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using optional chaining for safer container access.
The container access could be made safer by using optional chaining, especially since it's used in a try-catch block.
Apply this diff to improve the code:
-const beastiesContainer = document.beastiesContainer;
+const beastiesContainer = document?.beastiesContainer;
-return beastiesContainer.exists(sel);
+return beastiesContainer?.exists(sel);Also applies to: 582-582
| // beasties can be part of "legal comments" which aren't striped on build | ||
| const beastiesComment = rule.text.match(/^(?<!\! )beasties:(.*)/); | ||
| const command = beastiesComment && beastiesComment[1]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify the regex pattern for better readability.
The current regex pattern uses a negative lookbehind which can be complex to maintain. Consider using a simpler pattern that achieves the same result.
Apply this diff to simplify the regex:
-const beastiesComment = rule.text.match(/^(?<!\! )beasties:(.*)/);
+const beastiesComment = rule.text.match(/^(?!\!).*beasties:(.*)/);This pattern is more readable while still preventing matches of "! beasties:" comments.
Committable suggestion was skipped due to low confidence.
🧰 Tools
🪛 Biome
[error] 505-505: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (7)
package.json (1)
108-110: Consider documenting the resolutions field usage.The addition of the resolutions field is correct for yarn workspaces, but it would be helpful to document this in the README to explain its purpose for future contributors.
packages/beasties-webpack-plugin/README.md (3)
2-2: Consider hosting the logo image in the repository.The logo image is currently hosted on imgur, which could be unreliable for long-term maintenance. Consider moving it to the repository's assets directory for better control and availability.
10-12: Minor style improvements suggested.The content is technically accurate and well-structured, but consider these style improvements:
- Add "the" before "tradeoff": "This the tradeoff allows..."
- Rephrase to avoid split infinitive: "combining to improve dramatically" instead of "to dramatically improve"
🧰 Tools
🪛 LanguageTool
[formatting] ~10-~10: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...from other options, because it doesn't use a headless browser t...(COMMA_BEFORE_BECAUSE)
[uncategorized] ~10-~10: You might be missing the article “the” here.
Context: ...to render content. This tradeoff allows Beasties to be very fast and lightweight. It...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[style] ~12-~12: Style-wise, it’s not ideal to insert an adverb (‘dramatically’) in the middle of an infinitive construction (‘to improve’). Try moving the adverb to avoid split infinitives.
Context: ...com/GoogleChromeLabs/prerender-loader), combining to dramatically improve first paint time for most Single Page Applications. ## ...(SPLIT_INFINITIVE)
39-41: Enhance configuration example documentation.Consider expanding the "optional configuration" comment to include common configuration options or link directly to the configuration section. This would make it easier for users to get started.
new Beasties({ - // optional configuration (see below) + // Optional configuration options: + // preload: 'swap', // Controls how stylesheets are loaded + // preloadFonts: true, // Controls font loading behavior + // See full options at: [link to config section] })packages/beasties/README.md (3)
10-10: Remove unnecessary comma before 'because'.For better readability and grammar, remove the comma before "because" as the clause is essential to the meaning.
-It's a little different from [other options](#similar-libraries), because it **doesn't use a headless browser** to render content. +It's a little different from [other options](#similar-libraries) because it **doesn't use a headless browser** to render content.🧰 Tools
🪛 LanguageTool
[formatting] ~10-~10: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...from other options, because it doesn't use a headless browser t...(COMMA_BEFORE_BECAUSE)
178-185: Improve readability with proper punctuation and hyphenation.Several minor formatting improvements would enhance readability:
-By default Beasties evaluates +By default, Beasties evaluates -Usually this works well +Usually, this works well -To overcome this issue Beasties makes +To overcome this issue, Beasties makes -top level container +top-level container🧰 Tools
🪛 LanguageTool
[uncategorized] ~178-~178: Did you mean: “By default,”?
Context: ...ude end */ ``` ### Beasties container By default Beasties evaluates the CSS against the ...(BY_DEFAULT_COMMA)
[typographical] ~179-~179: Consider adding a comma after ‘Usually’ for more clarity.
Context: ...e CSS selectors to find matching nodes. Usually this works well as Beasties is lightwei...(RB_LY_COMMA)
[style] ~181-~181: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ... For some cases, the input HTML can be very large or deeply nested which makes the recons...(EN_WEAK_ADJECTIVE)
[grammar] ~181-~181: Did you mean “are” or “were”?
Context: ...n the critical CSS generation. Beasties is not aware of viewport size and what spe...(SENT_START_NNS_IS)
[uncategorized] ~183-~183: Possible missing comma found.
Context: ...ess browser involved. To overcome this issue Beasties makes use of **Beasties contai...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~185-~185: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...dingdata-beasties-containerinto the top level container thats contains the HTML eleme...(EN_COMPOUND_ADJECTIVE_INTERNAL)
Update remaining references to 'critters' in CHANGELOG.md
Found references to the old package name that need to be updated:
- In CHANGELOG.md:
critterspackage link in version header- Reference to
data-critters-containerin changelog entry🔗 Analysis chain
Line range hint
1-270: Verify complete migration from 'critters' to 'beasties'.Let's ensure all references to the old package name have been updated.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to "critters" (case-insensitive) # excluding the line mentioning the original fork rg -i "critter" | grep -v "GoogleChromeLabs/critters"Length of output: 263
🧰 Tools
🪛 LanguageTool
[grammar] ~6-~6: Did you mean “are” or “were”?
> Beasties is a plugin that inlines your app's [criti...
Context: ..."center">Beasties(SENT_START_NNS_IS)
[formatting] ~10-~10: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...from other options, because it doesn't use a headless browser t...(COMMA_BEFORE_BECAUSE)
[style] ~12-~12: Style-wise, it’s not ideal to insert an adverb (‘dramatically’) in the middle of an infinitive construction (‘to improve’). Try moving the adverb to avoid split infinitives.
Context: ...com/GoogleChromeLabs/prerender-loader), combining to dramatically improve first paint time for most Single Page Applications. ## ...(SPLIT_INFINITIVE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
- LICENSE (2 hunks)
- package.json (5 hunks)
- packages/beasties-webpack-plugin/README.md (3 hunks)
- packages/beasties-webpack-plugin/package.json (3 hunks)
- packages/beasties-webpack-plugin/src/index.js (3 hunks)
- packages/beasties/README.md (10 hunks)
✅ Files skipped from review due to trivial changes (1)
- LICENSE
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/beasties-webpack-plugin/src/index.js
🧰 Additional context used
🪛 LanguageTool
packages/beasties-webpack-plugin/README.md
[formatting] ~10-~10: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...from other options, because it doesn't use a headless browser t...(COMMA_BEFORE_BECAUSE)
[uncategorized] ~10-~10: You might be missing the article “the” here.
Context: ...to render content. This tradeoff allows Beasties to be very fast and lightweight. It...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[style] ~12-~12: Style-wise, it’s not ideal to insert an adverb (‘dramatically’) in the middle of an infinitive construction (‘to improve’). Try moving the adverb to avoid split infinitives.
Context: ...com/GoogleChromeLabs/prerender-loader), combining to dramatically improve first paint time for most Single Page Applications. ## ...(SPLIT_INFINITIVE)
packages/beasties/README.md
[grammar] ~6-~6: Did you mean “are” or “were”?
> Beasties is a plugin that inlines your app's [criti...
Context: ..."center">Beasties(SENT_START_NNS_IS)
[formatting] ~10-~10: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...from other options, because it doesn't use a headless browser t...(COMMA_BEFORE_BECAUSE)
[style] ~12-~12: Style-wise, it’s not ideal to insert an adverb (‘dramatically’) in the middle of an infinitive construction (‘to improve’). Try moving the adverb to avoid split infinitives.
Context: ...com/GoogleChromeLabs/prerender-loader), combining to dramatically improve first paint time for most Single Page Applications. ## ...(SPLIT_INFINITIVE)
[grammar] ~61-~61: Did you mean “are” or “were”?
Context: ...>" ``` ## Usage with webpack Beasties is also available as a Webpack plugin call...(SENT_START_NNS_IS)
[uncategorized] ~178-~178: Did you mean: “By default,”?
Context: ...ude end */ ``` ### Beasties container By default Beasties evaluates the CSS against the ...(BY_DEFAULT_COMMA)
[typographical] ~179-~179: Consider adding a comma after ‘Usually’ for more clarity.
Context: ...e CSS selectors to find matching nodes. Usually this works well as Beasties is lightwei...(RB_LY_COMMA)
[style] ~181-~181: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ... For some cases, the input HTML can be very large or deeply nested which makes the recons...(EN_WEAK_ADJECTIVE)
[grammar] ~181-~181: Did you mean “are” or “were”?
Context: ...n the critical CSS generation. Beasties is not aware of viewport size and what spe...(SENT_START_NNS_IS)
[uncategorized] ~183-~183: Possible missing comma found.
Context: ...ess browser involved. To overcome this issue Beasties makes use of **Beasties contai...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~185-~185: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...dingdata-beasties-containerinto the top level container thats contains the HTML eleme...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint
packages/beasties-webpack-plugin/README.md
54-54: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
packages/beasties/README.md
203-203: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
🔇 Additional comments (12)
packages/beasties-webpack-plugin/package.json (5)
28-31: LGTM! Contributor information properly added.The new contributor entry is well-formatted and includes all necessary fields.
5-11: Verify build output paths.The entry points and exports have been updated to use the new "beasties" naming. Please ensure that the build script in
scripts.buildwill generate files with these exact names.#!/bin/bash # Check if microbundle config or build script needs updating rg -l "critters-webpack-plugin" --type json
62-62: Review dependency version compatibility.The dependency on
[email protected]seems unusual given this package's version (3.0.2). This could indicate:
- A potential version mismatch
- The need to update to a stable version of the
beastiespackage#!/bin/bash # Check if beasties package exists and its available versions npm view beasties versions || echo "Package not found"
43-45: Verify repository configuration.The repository URL has been updated to point to the new location. Please ensure:
- The repository exists and is accessible
- The package directory structure matches the specified directory path
✅ Verification successful
Repository configuration is valid and correctly structured
The repository exists at github.com/danielroe/beasties and is actively maintained with multiple branches and PRs. The package directory structure matches the specified path at
packages/beasties-webpack-plugin/, confirming the correct monorepo setup.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify repository existence and structure gh repo view danielroe/beastiesLength of output: 283
Script:
#!/bin/bash # Let's verify if the repository exists using git ls-remote git ls-remote https://github.com/danielroe/beasties # Also check the directory structure fd --type directory "beasties-webpack-plugin" --max-depth 2Length of output: 625
2-4: Consider bumping the major version number.This rebranding from "critters" to "beasties" represents a significant change that could affect existing imports and usage. Following semver conventions, this warrants a major version bump (e.g., to 4.0.0).
package.json (3)
2-2: LGTM! Package metadata updates are consistent.The package name change and contributor addition are properly formatted and align with the rebranding effort.
Also applies to: 15-19
52-52: Verify test configuration completeness.The Jest configuration has been updated for the new package names. However, let's verify that all test-related paths are covered and no old references remain.
#!/bin/bash # Check for any test files still referencing old package names rg -l 'critters' --type test # Check for any remaining test configuration files fd -e test.js -e spec.jsAlso applies to: 59-60
35-36: Verify all workspace references are updated.While the build scripts have been updated to use the new workspace names, let's verify there are no remaining references to the old package names in other workspace-related configurations.
✅ Verification successful
Let me gather more information to verify workspace references.
All workspace references have been properly updated
All workspace-related configurations are correctly using the new package names:
- Root package.json correctly references
beastiesandbeasties-webpack-pluginin workspace scripts- Package dependencies and resolutions are updated
- Workspace configurations in package.json files are consistent with the rebranding
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to 'critters' in workspace configurations rg -i 'critters' --type jsonLength of output: 273
Script:
#!/bin/bash # Search for 'critters' in JSON files rg -i 'critters' --type json # Check package.json files in workspaces fd package.json -x cat {}Length of output: 6206
packages/beasties-webpack-plugin/README.md (1)
Line range hint
77-89: LGTM! Well-documented alternatives and licensing.The Similar Libraries section provides valuable context for users evaluating different solutions, and the licensing information is clear and complete.
🧰 Tools
🪛 LanguageTool
[formatting] ~10-~10: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...from other options, because it doesn't use a headless browser t...(COMMA_BEFORE_BECAUSE)
[uncategorized] ~10-~10: You might be missing the article “the” here.
Context: ...to render content. This tradeoff allows Beasties to be very fast and lightweight. It...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[style] ~12-~12: Style-wise, it’s not ideal to insert an adverb (‘dramatically’) in the middle of an infinitive construction (‘to improve’). Try moving the adverb to avoid split infinitives.
Context: ...com/GoogleChromeLabs/prerender-loader), combining to dramatically improve first paint time for most Single Page Applications. ## ...(SPLIT_INFINITIVE)
packages/beasties/README.md (3)
14-33: LGTM! Clear features list and installation instructions.The features are well-documented, and the installation instructions are accurate for both npm and yarn.
Line range hint
35-77: LGTM! Clear and comprehensive usage examples.Both the simple and webpack examples effectively demonstrate the usage patterns and configuration options.
🧰 Tools
🪛 LanguageTool
[grammar] ~61-~61: Did you mean “are” or “were”?
Context: ...>" ``` ## Usage with webpack Beasties is also available as a Webpack plugin call...(SENT_START_NNS_IS)
268-270: LGTM! References are accurate and properly maintained.All links are correctly updated to reflect the new package name and repository structure.
🧰 Tools
🪛 Markdownlint
270-270: Unused link or image reference definition: "html-webpack-plugin"
Link and image reference definitions should be needed(MD053, link-image-reference-definitions)
|
|
||
| #### Parameters | ||
|
|
||
| * `options` **Options** Options to control how Critters inlines CSS. See <https://github.com/GoogleChromeLabs/critters#usage> | ||
| * `options` **Options** Options to control how Beasties inlines CSS. See <https://github.com/danielroe/beasties#simple-example> |
There was a problem hiding this comment.
💡 Codebase verification
Update configuration options link to point to the correct documentation section.
The link to configuration options in the webpack plugin's README should point to the comprehensive configuration documentation in the main Beasties package instead of the simple example. The correct link should be https://github.com/danielroe/beasties#usage which contains the full configuration options documentation.
🔗 Analysis chain
Update configuration options link.
The link to configuration options (https://github.com/danielroe/beasties#simple-example) should point to a comprehensive configuration documentation section rather than just a simple example.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify if the linked section exists in the main README
rg -l "simple-example" "README.md"
Length of output: 32
Script:
#!/bin/bash
# Let's check if the repository has any README files and their content
fd -t f "README.md" -x cat {}
Length of output: 16164
danielroe
force-pushed
the
refactor/beasties
branch
from
78a51f5 to
12c1a33
Compare
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores