Skip to content

Conversation

@dschmidt
Copy link
Contributor

@dschmidt dschmidt commented Oct 22, 2025

…rameter list of amd modules

Description

I was assuming parameters were always wrapped in double quotes, but esbuild seems to use single quotes which is used to prebundle deps.

Related Issue

  • Fixes <issue_link>

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Types of changes

  • Bugfix
  • Enhancement (a change that doesn't break existing code or deployments)
  • Breaking change (a modification that affects current functionality)
  • Technical debt (addressing code that needs refactoring or improvements)
  • Tests (adding or improving tests)
  • Documentation (updates or additions to documentation)
  • Maintenance (like dependency updates or tooling adjustments)

Copilot AI review requested due to automatic review settings October 22, 2025 12:11
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 a bug in the AMD module wrapper plugin where it only detected the require dependency when wrapped in double quotes, but esbuild uses single quotes when prebundling dependencies.

Key Changes:

  • Updated the require dependency detection to handle both single and double quote wrapping

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

return {
code: code.replace(AmdWrapRE, (_, deps, params) => {
if (deps?.includes('"require"')) {
if (deps?.includes('"require"') || deps?.includes("'require'")) {
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

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

[nitpick] The condition checks for both quote styles but could be more maintainable using a regex pattern. Consider replacing with /(["'])require\1/.test(deps) to match 'require' with either quote style while ensuring consistency.

Suggested change
if (deps?.includes('"require"') || deps?.includes("'require'")) {
if (deps && /(["'])require\1/.test(deps)) {

Copilot uses AI. Check for mistakes.
@dschmidt dschmidt force-pushed the fix/extension-sdk-asset-imports branch from a2aba73 to d8d9210 Compare October 22, 2025 12:38
@dschmidt dschmidt force-pushed the fix/extension-sdk-asset-imports branch from d8d9210 to afb9fa5 Compare October 22, 2025 15:14
@JammingBen JammingBen merged commit 1101991 into opencloud-eu:main Oct 23, 2025
28 checks passed
@openclouders openclouders mentioned this pull request Oct 23, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants