Skip to content

Conversation

@marklawlor
Copy link
Contributor

No description provided.

Copilot AI review requested due to automatic review settings September 10, 2025 04:52
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the direct peer dependency on lightningcss and instead dynamically loads it from the @expo/metro-config package, falling back to direct resolution if needed. This change allows the project to use the version of lightningcss provided by Expo's configuration rather than requiring users to install it separately.

Key changes:

  • Added a new lightningcssLoader utility to dynamically resolve and load lightningcss
  • Updated the compiler to use the dynamically loaded version instead of direct imports
  • Enhanced style rule mapping propagation throughout the compilation process

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/compiler/lightiningcss-loader.ts New loader utility that resolves lightningcss from @expo/metro-config or fallback
src/compiler/compiler.ts Updated to use dynamic lightningcss loading and improved mapping propagation
src/compiler/selector-builder.ts Enhanced root variable selector detection to handle nesting
package.json Removed lightningcss as a peer dependency
Test files Updated test expectations for new compilation behavior

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@@ -0,0 +1,40 @@
export function lightningcssLoader() {
Copy link

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

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

The filename contains a typo: 'lightiningcss-loader.ts' should be 'lightningcss-loader.ts' (missing 'n' in 'lightning').

Copilot uses AI. Check for mistakes.
import { parseDeclaration, round } from "./declarations";
import { inlineVariables } from "./inline-variables";
import { extractKeyFrames } from "./keyframes";
import { lightningcssLoader } from "./lightiningcss-loader";
Copy link

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

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

The import path contains a typo: './lightiningcss-loader' should be './lightningcss-loader' (missing 'n' in 'lightning').

Suggested change
import { lightningcssLoader } from "./lightiningcss-loader";
import { lightningcssLoader } from "./lightningcss-loader";

Copilot uses AI. Check for mistakes.
@marklawlor marklawlor force-pushed the 09-10-fix_remove_lightningcss_as_a_peerdep._use_the_version_provided_by_expo branch from 7e956b3 to 8ff7f69 Compare September 10, 2025 04:54
@marklawlor marklawlor force-pushed the 09-10-fix_remove_lightningcss_as_a_peerdep._use_the_version_provided_by_expo branch from 8ff7f69 to c58ad12 Compare September 10, 2025 04:57
Copy link
Contributor Author

marklawlor commented Sep 10, 2025

Merge activity

  • Sep 10, 4:58 AM UTC: Graphite rebased this pull request as part of a merge.
  • Sep 10, 5:00 AM UTC: @marklawlor merged this pull request with Graphite.

@marklawlor marklawlor merged commit 15475e8 into main Sep 10, 2025
7 checks passed
@marklawlor marklawlor deleted the 09-10-fix_remove_lightningcss_as_a_peerdep._use_the_version_provided_by_expo branch September 10, 2025 05:00
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.

2 participants