Skip to content

fix: [] adaptive-expression does not work in the browser anymore#574

Closed
ceciliaavila wants to merge 5 commits intomainfrom
southworks/fix/browser-build
Closed

fix: [] adaptive-expression does not work in the browser anymore#574
ceciliaavila wants to merge 5 commits intomainfrom
southworks/fix/browser-build

Conversation

@ceciliaavila
Copy link

@ceciliaavila ceciliaavila commented Nov 11, 2025

Fixes # 4907
#minor

Description

This PR fixes the browser build for adaptive-expressions by disabling the tsup minification and using terser as a minifier in a separated build step.
It also adds a replacement for the assert module, required by the ANTLR parser.

Specific Changes

  • Added build:browser:min script to use terser as the adaptive-expressions minifier.
  • Added assert alias in tsup.config.ts.
  • Updated tsup.config.ts to allow overriding the minify option.

Testing

These images show the thee browser-compiled packages working after the changes.
image

@coderabbitai
Copy link

coderabbitai bot commented Nov 11, 2025

📝 Walkthrough

Walkthrough

Split the browser build into clean/run/minify steps; added terser minification and XML parser/xmldom dependencies to adaptive-expressions; changed build:browser:run to run without minification; added assert alias and disabled its polyfill in the browser bundler config.

Changes

Cohort / File(s) Summary
Adaptive Expressions package.json
libraries/adaptive-expressions/package.json
Added dependencies: @types/xmldom, @xmldom/xmldom, fast-xml-parser; added devDependency terser. Added script build:browser:min; reworked build:browser to run build:browser:clean, build:browser:run (now uses --no-minify), then build:browser:min (runs terser lib/browser.js --compress --output lib/browser.js). depcheck ignore list now includes terser.
Root package.json (devDeps reorder)
package.json
Adjusted devDependencies: added assert (dev), reordered/moved https-browserify and mocha-junit-reporter, consolidated typedoc entries; no runtime API changes.
Browser build config (tsup)
tsup/browser.config.ts
minify is now conditional (options.minify !== false) instead of always true; added esbuild alias assert: 'assert'; set esbuildPlugins.polyfills.assert = false to disable the assert polyfill for browser builds.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Dev as Developer
    participant NPM as npm scripts
    participant TSUP as tsup / bundler
    participant TERSER as terser
    rect rgb(230,245,255)
    Dev->>NPM: npm run build:browser
    end
    NPM->>NPM: build:browser:clean
    NPM->>TSUP: build:browser:run (--no-minify)
    TSUP->>TSUP: bundle -> produce lib/browser.js
    NPM->>TERSER: build:browser:min (terser lib/browser.js --compress)
    TERSER->>TERSER: minify & overwrite lib/browser.js
    TERSER->>Dev: finished
Loading
sequenceDiagram
    autonumber
    participant TSUP as tsup config
    participant ESBUILD as esbuild plugin
    rect rgb(255,250,230)
    TSUP->>ESBUILD: apply esbuildOptions.alias
    note right of ESBUILD `#e8f5e9`: alias: { assert: 'assert' }
    ESBUILD->>ESBUILD: apply polyfills
    note right of ESBUILD `#fbe9e7`: polyfills.assert = false
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify build:browser script sequencing and --no-minify on build:browser:run.
  • Confirm terser invocation overwrites lib/browser.js as intended.
  • Check compatibility and versions for @xmldom/xmldom, @types/xmldom, and fast-xml-parser.
  • Validate assert alias and polyfills.assert = false behavior in browser bundles.

Poem

🐰 I hop through scripts with tiny feet,

Clean, run, then minify — tidy and neat.
Parsers join the burrow, xmldom at the gate,
Assert tucked out of the polyfill crate.
A rabbit claps; the build is complete.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main purpose of the PR: fixing the browser build for adaptive-expressions package.
Description check ✅ Passed The description is clearly related to the changeset, explaining the fix, specific changes, and providing testing evidence.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch southworks/fix/browser-build

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4c3d845 and cfc4caf.

📒 Files selected for processing (1)
  • libraries/adaptive-expressions/package.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • libraries/adaptive-expressions/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: test:consumer
  • GitHub Check: tests.yml windows (node 22.x)
  • GitHub Check: tests.yml ubuntu (node 22.x)
  • GitHub Check: tests.yml ubuntu (node 18.x)
  • GitHub Check: tests.yml windows (node 20.x)
  • GitHub Check: tests.yml windows (node 18.x)
  • GitHub Check: tests.yml ubuntu (node 20.x)
  • GitHub Check: test:repoutils
  • GitHub Check: test:compat
  • GitHub Check: depcheck
  • GitHub Check: lint
  • GitHub Check: test:schemas

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 45d0427 and 92f9dab.

📒 Files selected for processing (3)
  • libraries/adaptive-expressions/package.json (1 hunks)
  • package.json (3 hunks)
  • tsup/browser.config.ts (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: test:consumer
  • GitHub Check: lint
  • GitHub Check: test:compat
  • GitHub Check: test:repoutils
  • GitHub Check: tests.yml ubuntu (node 22.x)
  • GitHub Check: tests.yml windows (node 22.x)
  • GitHub Check: tests.yml windows (node 20.x)
  • GitHub Check: tests.yml ubuntu (node 20.x)
  • GitHub Check: tests.yml windows (node 18.x)
  • GitHub Check: tests.yml ubuntu (node 18.x)
  • GitHub Check: test:schemas
  • GitHub Check: depcheck
🔇 Additional comments (2)
tsup/browser.config.ts (1)

43-43: Assert module configuration looks correct.

The alias mapping at line 43 correctly resolves assert imports to the assert package, and the polyfill configuration at line 56 appropriately disables polyfilling since an explicit alias is provided. This approach is consistent with how other Node modules (crypto, http, https, stream) are handled in the browser build.

Also applies to: 56-56

package.json (1)

96-96: No issues found—version 2.1.0 is current and secure.

The latest version of the assert package is 2.1.0, which matches what's specified in package.json. No security vulnerabilities were found for this package. The dependency addition is appropriate and safe.

@ceciliaavila
Copy link
Author

/promoted 4909

@ceciliaavila ceciliaavila deleted the southworks/fix/browser-build branch November 12, 2025 16:11
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