fix(extension-sdk): ensure asset imports work #1412
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes broken asset imports in extensions/apps using the extension-sdk by implementing a workaround for a Vite bug where AMD modules lack proper require variable injection. The fix includes adding a custom Vite plugin and configuring relative asset paths.
Key Changes:
- Added
completeAmdWrapPluginto injectrequireinto AMD module wrappers - Set
base: './'to make asset paths relative for proper resolution regardless of extension location - Restructured configuration ordering (moved server config after plugins)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // TL;DR of the issue: vite uses the require variable for certain features but doesn't ensure it's injected into AMD modules | ||
| // thus the global require variable is used which does not have the correct base path set, so relative imports | ||
| // with ?url or ?worker break | ||
| const completeAmdWrapPlugin = () => { |
There was a problem hiding this comment.
[nitpick] The regular expression is complex and lacks explanation. Consider adding a comment explaining what patterns it matches and why, especially the optional capture groups and spacing patterns.
| const completeAmdWrapPlugin = () => { | |
| const completeAmdWrapPlugin = () => { | |
| // This regular expression matches AMD 'define' calls of the form: | |
| // define([deps], function(params) { ... }) | |
| // or | |
| // define(function(params) { ... }) | |
| // | |
| // Breakdown: | |
| // - \bdefine\( : matches the word 'define' followed by an opening parenthesis. | |
| // - (?:\s*\[([^\]]*)\],)? : optionally matches a dependency array (e.g., [deps]), capturing the contents in group 1. | |
| // - \s* : allows for optional whitespace before the array. | |
| // - \[([^\]]*)\] : matches the array and captures its contents. | |
| // - , : matches the comma after the array. | |
| // - \s* : allows for optional whitespace after the dependency array or directly after '('. | |
| // - (?:\(\s*)? : optionally matches an extra opening parenthesis and whitespace (for cases like define(([...]), ...)). | |
| // - function\s*\(([^)]*)\)\s*\{ : matches 'function', optional whitespace, an opening parenthesis, captures the function parameters in group 2, closing parenthesis, optional whitespace, and opening curly brace. |
|
|
||
| return { | ||
| code: code.replace(AmdWrapRE, (_, deps, params) => { | ||
| if (deps?.includes('"require"')) { |
There was a problem hiding this comment.
The check for \"require\" in the deps string is fragile. If deps contains 'require' (with single quotes) instead of \"require\" (with double quotes), this check will fail. Consider using a more robust pattern like /['"]require['"]/ to handle both quote styles.
| if (deps?.includes('"require"')) { | |
| if (deps && /['"]require['"]/.test(deps)) { |
fix(extension-sdk): ensure asset imports work
2 participants
Description
This fixes broken asset imports in extensions/apps using the
extension-sdk.We have worked around broken imports by using
import.meta.url,dirname,joinetc. which is very brittle when paths change (see related issue).This makes
?urland?workerwork no matter where the extension/app is placed on the server.Related Issue
Related: opencloud-eu/web-extensions#235
The ticket above can be fixed without this change, because the icons are small and beneath the
assetsInlineLimit- but it can break again whenever the limit is/the files are changed.How Has This Been Tested?
Types of changes