Skip to content

Comments

fix(export): resolve npm packages via node module resolution in virtual content loader#346

Merged
pasevin merged 2 commits intomainfrom
fix/virtual-content-loader-npm-resolve
Feb 20, 2026
Merged

fix(export): resolve npm packages via node module resolution in virtual content loader#346
pasevin merged 2 commits intomainfrom
fix/virtual-content-loader-npm-resolve

Conversation

@pasevin
Copy link
Collaborator

@pasevin pasevin commented Feb 20, 2026

Summary

  • Fix empty global.css in staging exports caused by the virtual content loader's hardcoded node_modules/ path failing under pnpm's isolated node-linker
  • The .npmrc (which contains shamefully-hoist=true) is excluded from Docker builds via .dockerignore. Without it, pnpm defaults to isolated mode where @openzeppelin/ui-styles is only available in apps/builder/node_modules/, not at the monorepo root — causing the virtual content loader to silently fall back to an empty string
  • Instead of exposing .npmrc to Docker (which could leak future auth tokens), introduce an npm: prefix convention in the virtualFiles map that triggers createRequire() resolution from the builder package directory, which correctly follows pnpm symlinks in any hoisting strategy

Before

'global-css-content': 'node_modules/@openzeppelin/ui-styles/global.css'
// ❌ Only works with shamefully-hoist=true (hoisted layout)

After

'global-css-content': 'npm:@openzeppelin/ui-styles/global.css'
// ✅ Uses Node module resolution — works with any pnpm node-linker

Test plan

  • Merge and verify the staging export produces a non-empty src/styles/global.css
  • Run pnpm build on the exported app and verify bg-background resolves
  • Verify local pnpm build of the builder app still embeds CSS correctly

…al content loader

The virtual content loader hardcoded a node_modules/ path for
@openzeppelin/ui-styles/global.css, which only works when pnpm
uses shamefully-hoist (hoisted node-linker). In Docker builds
where .npmrc is excluded, pnpm defaults to isolated mode and the
package is not hoisted to the root node_modules, causing the CSS
theme to silently resolve as empty.

Use createRequire from the builder package directory to resolve
npm: prefixed entries via Node's module resolution algorithm,
which correctly follows pnpm symlinks in any hoisting strategy.
@pasevin pasevin requested a review from a team as a code owner February 20, 2026 14:32
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 fixes an issue where the global.css file was empty in staging exports when using pnpm's isolated node-linker strategy. The fix introduces an npm: prefix convention in the virtual content loader plugin that uses Node's module resolution via createRequire(), which correctly handles pnpm symlinks regardless of hoisting strategy.

Changes:

  • Introduced npm: prefix convention for npm package resolution in virtual files map
  • Added createRequire() from Node's module system to resolve npm packages from the builder directory
  • Updated global-css-content entry to use npm:@openzeppelin/ui-styles/global.css instead of hardcoded node_modules/ path

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The .replace(/\\/g, '\\') call was a no-op (replacing backslash
with itself). Fix to .replace(/\\/g, '\\\\') so backslashes in
CSS content are properly escaped when embedded in template
literals. Flagged by CodeQL.
@pasevin pasevin force-pushed the fix/virtual-content-loader-npm-resolve branch from 78c9b3a to 51b3815 Compare February 20, 2026 14:56
@pasevin pasevin merged commit b1ff983 into main Feb 20, 2026
11 checks passed
@pasevin pasevin deleted the fix/virtual-content-loader-npm-resolve branch February 20, 2026 15:04
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.

1 participant