feat(parcel): add Parcel support #71
Conversation
🦋 Changeset detectedLatest commit: a154823 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds a new Parcel transformer package Changes
Sequence Diagram(s)sequenceDiagram
participant Parcel as Parcel
participant Transformer as parcel-transformer-lit-css
participant Lib as `@pwrs/lit-css`
participant Asset as CSS Asset
rect rgb(230,245,255)
Parcel->>Transformer: loadConfig({ config })
Transformer-->>Parcel: return package "lit-css" config (if present)
end
rect rgb(245,255,230)
Parcel->>Transformer: transform({ asset, config })
Transformer->>Asset: asset.getCode()
Asset-->>Transformer: CSS content
Transformer->>Lib: transform({ css, filePath, options })
Lib-->>Transformer: JS module code (lit `css` template)
Transformer->>Asset: asset.setCode(transformed JS)
Transformer->>Asset: asset.type = "js"
Transformer-->>Parcel: return [updated asset]
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/parcel-transformer-lit-css/README.md (1)
95-111: Update Sass example to use modern API.The preprocessor example uses the deprecated
Sass.render()callback-based API. Modern versions of the Sass package usecompile()orcompileString()instead.Apply this diff to update the example:
// custom-transform.js -import Sass from 'sass'; +import * as sass from 'sass'; export default async function sassTransform(css, { filePath }) { - return new Promise((resolve, reject) => { - Sass.render({ data: css, file: filePath }, (err, result) => { - if (err) reject(err); - else resolve(result.css.toString()); - }); - }); + try { + const result = sass.compileString(css, { url: new URL(`file://${filePath}`) }); + return result.css; + } catch (err) { + throw err; + } }packages/parcel-transformer-lit-css/parcel-transformer-lit-css.ts (2)
14-23: Consider adding runtime validation for config.The type assertion
(config as LitCSSOptions)bypasses type safety. While the transform function will handle invalid options gracefully, explicit validation would provide clearer error messages if the configuration is malformed.
29-30: Preserve the original error stack trace.Creating a new
Errordiscards the original stack trace, making debugging harder. Consider re-throwing the error directly or using thecauseoption.Apply this diff to preserve the stack trace:
- } catch (error: any) { - throw new Error(error?.message ?? String(error)); - } + } catch (error: any) { + throw error instanceof Error ? error : new Error(String(error)); + }packages/parcel-transformer-lit-css/package.json (1)
8-10: Consider adding TypeScript types to exports map.While the top-level
typesfield works, modern TypeScript resolves types better when they're included in the exports map alongside the import/require entries.Example enhancement:
"exports": { - "import": "./parcel-transformer-lit-css.js", - "require": "./parcel-transformer-lit-css.cjs" + "import": { + "types": "./parcel-transformer-lit-css.d.ts", + "default": "./parcel-transformer-lit-css.js" + }, + "require": { + "types": "./parcel-transformer-lit-css.d.ts", + "default": "./parcel-transformer-lit-css.cjs" + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpackages/parcel-transformer-lit-css/integration-test/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (24)
.changeset/sad-boats-tan.md(1 hunks).eslintignore(1 hunks).gitignore(1 hunks)README.md(1 hunks)package.json(1 hunks)packages/parcel-transformer-lit-css/.gitignore(1 hunks)packages/parcel-transformer-lit-css/README.md(1 hunks)packages/parcel-transformer-lit-css/integration-test/.parcelrc(1 hunks)packages/parcel-transformer-lit-css/integration-test/README.md(1 hunks)packages/parcel-transformer-lit-css/integration-test/package.json(1 hunks)packages/parcel-transformer-lit-css/integration-test/src/index.html(1 hunks)packages/parcel-transformer-lit-css/integration-test/src/my-element.js(1 hunks)packages/parcel-transformer-lit-css/integration-test/src/styles.css(1 hunks)packages/parcel-transformer-lit-css/package.json(1 hunks)packages/parcel-transformer-lit-css/parcel-transformer-lit-css.ts(1 hunks)packages/parcel-transformer-lit-css/test/expected/bare/output.js(1 hunks)packages/parcel-transformer-lit-css/test/expected/basic/boop.js(1 hunks)packages/parcel-transformer-lit-css/test/expected/basic/fast.js(1 hunks)packages/parcel-transformer-lit-css/test/expected/basic/output.js(1 hunks)packages/parcel-transformer-lit-css/test/expected/basic/uglified.js(1 hunks)packages/parcel-transformer-lit-css/test/expected/postcss/output.js(1 hunks)packages/parcel-transformer-lit-css/test/expected/scss/output.js(1 hunks)packages/parcel-transformer-lit-css/test/expected/special-chars/output.js(1 hunks)packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-03T06:29:44.714Z
Learnt from: bennypowers
Repo: bennypowers/lit-css PR: 70
File: packages/vite-plugin-lit-css/integration-test/src/my-element.js:5-5
Timestamp: 2025-11-03T06:29:44.714Z
Learning: In the bennypowers/lit-css repository, the .eslintignore file contains the pattern "packages/*/integration-test/**/*.js" which correctly excludes integration test files from linting, so ESLint parsing errors should not occur for files in integration-test directories.
Applied to files:
packages/parcel-transformer-lit-css/integration-test/package.jsonpackages/parcel-transformer-lit-css/integration-test/src/styles.csspackages/parcel-transformer-lit-css/.gitignore.eslintignorepackages/parcel-transformer-lit-css/integration-test/README.md.gitignorepackages/parcel-transformer-lit-css/integration-test/src/index.htmlpackages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.jsREADME.mdpackages/parcel-transformer-lit-css/integration-test/.parcelrc
🧬 Code graph analysis (10)
packages/parcel-transformer-lit-css/test/expected/basic/uglified.js (5)
packages/parcel-transformer-lit-css/test/expected/bare/output.js (1)
styles(2-5)packages/parcel-transformer-lit-css/test/expected/basic/fast.js (1)
styles(2-5)packages/parcel-transformer-lit-css/test/expected/basic/output.js (1)
styles(2-5)packages/parcel-transformer-lit-css/test/expected/postcss/output.js (1)
styles(2-2)packages/parcel-transformer-lit-css/test/expected/scss/output.js (1)
styles(2-2)
packages/parcel-transformer-lit-css/integration-test/src/my-element.js (2)
packages/parcel-transformer-lit-css/test/expected/bare/output.js (1)
styles(2-5)packages/parcel-transformer-lit-css/test/expected/basic/output.js (1)
styles(2-5)
packages/parcel-transformer-lit-css/test/expected/special-chars/output.js (2)
packages/parcel-transformer-lit-css/test/expected/bare/output.js (1)
styles(2-5)packages/parcel-transformer-lit-css/test/expected/basic/output.js (1)
styles(2-5)
packages/parcel-transformer-lit-css/test/expected/basic/boop.js (7)
packages/parcel-transformer-lit-css/test/expected/bare/output.js (1)
styles(2-5)packages/parcel-transformer-lit-css/test/expected/basic/fast.js (1)
styles(2-5)packages/parcel-transformer-lit-css/test/expected/basic/output.js (1)
styles(2-5)packages/parcel-transformer-lit-css/test/expected/basic/uglified.js (1)
styles(2-2)packages/parcel-transformer-lit-css/test/expected/postcss/output.js (1)
styles(2-2)packages/parcel-transformer-lit-css/test/expected/scss/output.js (1)
styles(2-2)packages/parcel-transformer-lit-css/test/expected/special-chars/output.js (1)
styles(2-8)
packages/parcel-transformer-lit-css/test/expected/bare/output.js (2)
packages/parcel-transformer-lit-css/test/expected/basic/fast.js (1)
styles(2-5)packages/parcel-transformer-lit-css/test/expected/basic/output.js (1)
styles(2-5)
packages/parcel-transformer-lit-css/test/expected/postcss/output.js (2)
packages/parcel-transformer-lit-css/test/expected/basic/output.js (1)
styles(2-5)packages/parcel-transformer-lit-css/test/expected/basic/uglified.js (1)
styles(2-2)
packages/parcel-transformer-lit-css/test/expected/basic/fast.js (3)
packages/parcel-transformer-lit-css/test/expected/bare/output.js (1)
styles(2-5)packages/parcel-transformer-lit-css/test/expected/basic/output.js (1)
styles(2-5)packages/parcel-transformer-lit-css/test/expected/basic/uglified.js (1)
styles(2-2)
packages/parcel-transformer-lit-css/test/expected/basic/output.js (3)
packages/parcel-transformer-lit-css/test/expected/bare/output.js (1)
styles(2-5)packages/parcel-transformer-lit-css/test/expected/basic/boop.js (1)
styles(2-5)packages/parcel-transformer-lit-css/test/expected/basic/fast.js (1)
styles(2-5)
packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js (1)
test/test.js (1)
run(24-108)
packages/parcel-transformer-lit-css/test/expected/scss/output.js (2)
packages/parcel-transformer-lit-css/test/expected/bare/output.js (1)
styles(2-5)packages/parcel-transformer-lit-css/test/expected/basic/uglified.js (1)
styles(2-2)
🪛 LanguageTool
packages/parcel-transformer-lit-css/integration-test/README.md
[grammar] ~51-~51: Ensure spelling is correct
Context: ...onent's static styles property 4. The hotpink color is applied to the h1 element You...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (25)
packages/parcel-transformer-lit-css/.gitignore (1)
1-1: LGTM!Appropriately ignores tarball artifacts generated by
npm packfor local testing workflows..eslintignore (1)
3-3: LGTM!Correctly excludes integration test JavaScript files from linting, consistent with the repository's approach to generated and test fixture code.
packages/parcel-transformer-lit-css/test/expected/basic/fast.js (1)
1-9: LGTM!Test fixture correctly demonstrates the expected output format for @microsoft/fast-element CSS transformations, with both default and named exports.
.gitignore (1)
115-115: LGTM!Appropriately excludes Parcel's cache directory from version control, consistent with other build tool caches in this file.
packages/parcel-transformer-lit-css/integration-test/README.md (1)
1-125: LGTM!Comprehensive integration test documentation that clearly explains the purpose, setup steps, and verification process. The instructions for both build and dev modes are well-structured and helpful.
packages/parcel-transformer-lit-css/test/expected/bare/output.js (1)
1-9: LGTM!Test fixture correctly demonstrates the expected output format for lit CSS transformations, maintaining consistency with other test expectations.
package.json (1)
36-38: Parcel dependencies verified as current and secure.The versions @parcel/core, @parcel/config-default, and @parcel/plugin at 2.16.0 are the latest available and have no known published vulnerabilities. A historical Origin Validation Error (CVE-2025-56648) affected older Parcel dev-server packages up through 2.14.4 and was fixed in later releases. The caret range specified (^2.16.0) is appropriate and safe.
packages/parcel-transformer-lit-css/parcel-transformer-lit-css.ts (1)
9-12: LGTM!The configuration loading logic correctly uses optional chaining to safely access the
lit-cssconfiguration frompackage.json.packages/parcel-transformer-lit-css/integration-test/src/styles.css (1)
1-9: LGTM!The CSS file provides a clear test case for the transformer with both
:hostand element selectors.packages/parcel-transformer-lit-css/integration-test/.parcelrc (1)
1-6: LGTM!The Parcel configuration correctly extends the default config and registers the transformer for CSS files.
README.md (1)
30-30: LGTM!The Parcel documentation entry is correctly positioned and formatted consistently with the existing plugin list.
packages/parcel-transformer-lit-css/test/expected/basic/uglified.js (1)
1-6: LGTM!The expected output correctly demonstrates the transformer's handling of minified CSS with proper export patterns.
packages/parcel-transformer-lit-css/integration-test/src/index.html (1)
1-16: LGTM!The integration test HTML is well-structured and correctly loads the module script to test the custom element with transformed CSS.
packages/parcel-transformer-lit-css/test/expected/basic/output.js (1)
1-9: LGTM!The expected output correctly demonstrates the transformer's handling of formatted CSS with the proper dual export pattern.
packages/parcel-transformer-lit-css/integration-test/package.json (1)
15-15: Integration test is a documented manual prerequisite, not a build process requirement.The verification shows that tarball generation is not configured in the build process or CI/CD workflows. However, this is intentional. The integration test README documents this as a manual prerequisite: "Before running the integration test, you need to pack the transformer package: cd .. # Go to parcel-transformer-lit-css directory; npm pack".
The root test script only runs automated tape tests, not integration tests. The CI/CD workflows (pull-request.yml and release.yml) both execute
npm test, which does not include integration tests. The integration test is designed as a manual verification step for developers, not part of the automated build pipeline.Likely an incorrect or invalid review comment.
.changeset/sad-boats-tan.md (1)
1-14: LGTM! Clear and comprehensive changeset documentation.The changeset clearly describes the new Parcel transformer package and its features.
packages/parcel-transformer-lit-css/integration-test/src/my-element.js (1)
1-15: LGTM! Clean integration test component.This Lit element correctly demonstrates the expected usage pattern for the transformer: importing a CSS file and assigning it to the static styles property.
packages/parcel-transformer-lit-css/test/expected/basic/boop.js (1)
1-9: LGTM! Test fixture for custom specifier/tag options.This fixture correctly tests the transformer's ability to use custom specifier and tag names (importing
boopfromsnootinstead of the defaultcssfromlit).packages/parcel-transformer-lit-css/test/expected/special-chars/output.js (1)
1-12: LGTM! Test fixture validates special character handling.This fixture appropriately tests that the transformer preserves special characters (emoji) and CSS directives (@import) within the transformed output.
packages/parcel-transformer-lit-css/test/expected/scss/output.js (1)
1-6: LGTM! Test fixture for SCSS transformation.This fixture correctly represents the expected output when transforming SCSS with minification enabled.
packages/parcel-transformer-lit-css/test/expected/postcss/output.js (1)
1-6: LGTM! Test fixture for PostCSS transformation.This fixture correctly represents the expected output when transforming CSS through PostCSS with minification.
packages/parcel-transformer-lit-css/package.json (1)
36-41: LGTM! Correct dependency structure.The dependency configuration is correct:
@parcel/pluginas a peer dependency (provided by Parcel runtime) and@pwrs/lit-cssas a regular dependency.packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js (3)
13-49: LGTM! Comprehensive test environment setup.The test correctly creates an isolated environment with symlinking to the root node_modules for dependency access, and copies the transformer source for local resolution by Parcel. This is a standard pattern for testing build plugins.
51-82: LGTM! Well-configured Parcel build for testing.The build configuration is appropriate: production mode with optimization disabled keeps output readable for test assertions, scopeHoist enables proper ES module output, and comprehensive error handling logs diagnostics for debugging.
84-102: LGTM! Robust bundle extraction and cleanup.The code defensively checks for empty bundles, finds the appropriate JS bundle, and ensures cleanup happens in the finally block even if errors occur.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js (1)
86-104: Minor formatting and bundle selection notes.The cleanup logic is solid with proper use of the
finallyblock. Two minor observations:
- Line 92: Empty line could be removed
- Line 94: Bundle selection uses
findfor JS bundles with fallback—this is adequate for the test cases but could be more explicit with a comment explaining the fallback scenarioApply this diff to remove the empty line:
if (bundles.length === 0) throw new Error('No bundles found in build output'); - // Get the main bundlepackages/parcel-transformer-lit-css/parcel-transformer-lit-css.ts (1)
29-31: Consider preserving the original error stack trace.Creating a new
Errorobject loses the stack trace from the original error, making debugging more difficult. Consider re-throwing the original error when possible.Apply this diff to preserve stack traces:
- } catch (error) { - throw new Error(error?.message ?? String(error)); - } + } catch (error) { + throw error instanceof Error ? error : new Error(String(error)); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/parcel-transformer-lit-css/parcel-transformer-lit-css.ts(1 hunks)packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: bennypowers
Repo: bennypowers/lit-css PR: 70
File: packages/vite-plugin-lit-css/integration-test/src/my-element.js:5-5
Timestamp: 2025-11-03T06:29:44.714Z
Learning: In the bennypowers/lit-css repository, the .eslintignore file contains the pattern "packages/*/integration-test/**/*.js" which correctly excludes integration test files from linting, so ESLint parsing errors should not occur for files in integration-test directories.
📚 Learning: 2025-11-03T06:29:44.714Z
Learnt from: bennypowers
Repo: bennypowers/lit-css PR: 70
File: packages/vite-plugin-lit-css/integration-test/src/my-element.js:5-5
Timestamp: 2025-11-03T06:29:44.714Z
Learning: In the bennypowers/lit-css repository, the .eslintignore file contains the pattern "packages/*/integration-test/**/*.js" which correctly excludes integration test files from linting, so ESLint parsing errors should not occur for files in integration-test directories.
Applied to files:
packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js
🧬 Code graph analysis (1)
packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js (1)
test/test.js (1)
run(24-108)
🔇 Additional comments (6)
packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js (3)
1-12: LGTM! Well-structured imports and setup.The imports are appropriate for the Parcel integration test, and the shared fixtures directory reference maintains consistency with the project structure.
53-84: Excellent Parcel configuration and error handling.The bundler configuration is well-suited for testing with
optimize: falseto maintain readable output for assertions, and the comprehensive error handling with diagnostic logging will make debugging transformer issues much easier.
22-25: The original review comment is based on an incorrect assumption.CI/CD only uses
ubuntu-latest(Linux)—no Windows runners are configured. The symlink approach in the test is appropriate for the Linux-only testing environment. Windows portability is not a blocking concern for this codebase.Likely an incorrect or invalid review comment.
packages/parcel-transformer-lit-css/parcel-transformer-lit-css.ts (3)
1-6: LGTM!The imports are appropriate and the
LitCSSOptionstype correctly omits thecssproperty since it will be provided by reading the asset content.
9-12: LGTM!The configuration loading correctly reads from
package.jsonand safely accesses thelit-cssconfiguration using optional chaining.
14-28: The transform function returns only a code string; verify if dependency tracking is necessary for this transformer's use case.The
transform()function in the local@pwrs/lit-csspackage (packages/lit-css/lit-css.ts) returnsPromise<string>containing only the transformed code—it does not return an object with dependency information. Therefore, the assumption that dependency info is being ignored is incorrect.However, the underlying concern about CSS imports remains valid if:
- CSS files can have
@importstatements that aren't resolved by the optional preprocessor- Parcel's CSS handler doesn't already track these imports before this transformer runs
To resolve this, verify that CSS files with
@importstatements are properly tracked by Parcel's built-in CSS handling, or test with a CSS file containing@importto confirm hot reloading and cache invalidation work correctly.The suggestion about source maps is still worth considering for improved debugging of CSS-to-JS transformations.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
package.json(1 hunks)packages/parcel-transformer-lit-css/README.md(1 hunks)packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js(1 hunks)test/mock-packages/README.md(1 hunks)test/mock-packages/snoot/index.js(1 hunks)test/mock-packages/snoot/package.json(1 hunks)test/test.js(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- test/mock-packages/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: bennypowers
Repo: bennypowers/lit-css PR: 70
File: packages/vite-plugin-lit-css/integration-test/src/my-element.js:5-5
Timestamp: 2025-11-03T06:29:44.714Z
Learning: In the bennypowers/lit-css repository, the .eslintignore file contains the pattern "packages/*/integration-test/**/*.js" which correctly excludes integration test files from linting, so ESLint parsing errors should not occur for files in integration-test directories.
📚 Learning: 2025-11-03T06:29:44.714Z
Learnt from: bennypowers
Repo: bennypowers/lit-css PR: 70
File: packages/vite-plugin-lit-css/integration-test/src/my-element.js:5-5
Timestamp: 2025-11-03T06:29:44.714Z
Learning: In the bennypowers/lit-css repository, the .eslintignore file contains the pattern "packages/*/integration-test/**/*.js" which correctly excludes integration test files from linting, so ESLint parsing errors should not occur for files in integration-test directories.
Applied to files:
packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.jstest/test.js
🧬 Code graph analysis (2)
packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js (1)
test/test.js (1)
run(24-114)
test/test.js (1)
packages/typescript-transform-lit-css/test/typescript-transform-lit-css.test.js (2)
getCode(18-19)read(15-16)
🪛 GitHub Actions: Verify PR
packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js
[warning] 5-5: eslint: 'mkdir' is defined but never used. (no-unused-vars) | Command: npm run lint
[error] 29-29: eslint: Use array destructuring. (prefer-destructuring) | Command: npm run lint
🪛 GitHub Check: verify
packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js
[failure] 183-183:
'originalCwd' is not defined
[failure] 176-176:
Unnecessary catch clause
[failure] 170-170:
Use array destructuring
[failure] 157-157:
Split 'let' declarations into multiple statements
[warning] 156-156:
'tag' is assigned a value but never used
[failure] 156-156:
Use array destructuring
[failure] 155-155:
Use array destructuring
[failure] 146-146:
'||' should be placed at the end of the line
[failure] 82-82:
Multiple spaces found before '// Now relativ...'
[warning] 78-78:
'originalCwd' is assigned a value but never used
[failure] 63-63:
Missing trailing comma
[warning] 35-35:
'input' is assigned a value but never used
[failure] 29-29:
Use array destructuring
[warning] 5-5:
'mkdir' is defined but never used
test/mock-packages/snoot/index.js
[warning] 2-2:
'values' is defined but never used
🔇 Additional comments (6)
test/mock-packages/snoot/package.json (1)
1-6: LGTM! Clean mock package manifest.The package.json correctly defines a test fixture with ESM module support, which aligns with the test requirements for custom specifier/tag combinations.
test/mock-packages/snoot/index.js (1)
1-4: LGTM! Correct signature for tagged template literal.The
valuesparameter is intentionally unused because this is a mock function for testing custom tag behavior. Tagged template literal functions require the(strings, ...values)signature even when the implementation doesn't need the interpolated values.test/test.js (2)
24-24: Good addition of skip parameter.The optional
skiparray enables selective test execution, which is necessary for different bundler implementations that handle preprocessing differently.
64-110: Well-structured conditional test execution.The skip guards for
scss-transform,scss-error, andpostcss-transformtests are correctly implemented. This allows Parcel to use its own transformer chain while other bundlers can still use the transform callbacks.packages/parcel-transformer-lit-css/README.md (1)
1-116: Excellent documentation.The README is comprehensive and well-organized, covering all essential aspects:
- Clear installation and setup instructions
- Practical usage examples
- Configuration options with defaults
- Input/output example for clarity
- Preprocessor integration guidance
This addresses the issue #44 requirement for sample configuration.
packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js (1)
14-189: Solid integration test approach.The test runner correctly:
- Creates an isolated environment with symlinked dependencies
- Configures Parcel with the custom transformer
- Handles different specifier/tag combinations
- Extracts and normalizes output for comparison
- Skips transform callback tests (using Parcel's transformer chain instead)
The CSS extraction logic intelligently handles Parcel's bundling patterns to verify the transformation.
packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js
Outdated
Show resolved
Hide resolved
packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js
Outdated
Show resolved
Hide resolved
packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js
Outdated
Show resolved
Hide resolved
packages/parcel-transformer-lit-css/test/parcel-transformer-lit-css.test.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
scripts/setup-mock-packages.js (2)
11-21: Validate target directory exists before creating symlink.The script creates a symlink without verifying that the target directory exists. If
test/mock-packages/snootis missing, a broken symlink will be created, leading to confusing errors when tests attempt to use the mock package.Apply this diff to validate the target before symlinking:
if (!existsSync(link)) { + if (!existsSync(target)) { + console.error('✗ Mock package directory does not exist:', target); + process.exit(1); + } try { symlinkSync(target, link, 'dir');
11-21: Consider cross-platform symlink limitations.Directory symlinks on Windows typically require administrator privileges. While the error handling allows the script to continue with a warning, developers on Windows without elevated privileges may encounter issues. Consider documenting this requirement or exploring alternatives like junction points (
'junction'type) which don't require admin rights on Windows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.json(2 hunks)scripts/setup-mock-packages.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: bennypowers
Repo: bennypowers/lit-css PR: 70
File: packages/vite-plugin-lit-css/integration-test/src/my-element.js:5-5
Timestamp: 2025-11-03T06:29:44.714Z
Learning: In the bennypowers/lit-css repository, the .eslintignore file contains the pattern "packages/*/integration-test/**/*.js" which correctly excludes integration test files from linting, so ESLint parsing errors should not occur for files in integration-test directories.
🪛 GitHub Actions: Verify PR
scripts/setup-mock-packages.js
[error] 14-14: ESLint: Unexpected console statement. (no-console)
🪛 GitHub Check: verify
scripts/setup-mock-packages.js
[failure] 18-18:
Unexpected console statement
[failure] 17-17:
Unnecessary { after 'if' condition
[failure] 14-14:
Unexpected console statement
|
@mediabuff, can you try this out? |
Closes #44
Summary by CodeRabbit
New Features
Documentation
Tests
Chores