Skip to content

Disable AMD in our bundle#5478

Merged
compulim merged 11 commits intomicrosoft:mainfrom
compulim:fix-requirejs
May 15, 2025
Merged

Disable AMD in our bundle#5478
compulim merged 11 commits intomicrosoft:mainfrom
compulim:fix-requirejs

Conversation

@compulim
Copy link
Copy Markdown
Contributor

@compulim compulim commented May 15, 2025

Fixes #5474.

Changelog Entry

Fixed

  • Fixed #5474. Disable AMD glue code in bundle, in PR #5478, by @compulim
    • Downstreamers who use our CommonJS and ES Modules output with esbuild will need to disable AMD themselves to prevent conflict with RequireJS

Description

RequireJS does not support anonymous modules via AMD. However, globalize-runtime, parse-srcset, etc. use anonymous modules in AMD via UMD. Thus, if Web Chat is loaded after RequireJS, it will fail as these packages tries to use AMD anonymously.

The root cause seems to be that esbuild don't touch/remove AMD glue code in our bundle. This is their design choice to leave the AMD world behind.

We can safely remove AMD glue code in our bundle as our bundle already has everything it needed.

For web devs who consume our CJS/ESM, they will need to remove AMD glue code in their bundler. For Web Chat, we emit CJS/ESM code as faithfully as the source code. That means we do minimal transpilation, e.g. no async to generator helper, no rest/spread helpers, etc.

It is up to web devs to transpile to their target. This design helps to reduce bundle size, less glue code and potentially better treeshaking, and more controls by the web devs.

To me, the root cause is some packages that is exposed as AMD following what the AMD spec said, but RequireJS has a more stringent requirement (no anonymous modules). Those packages are conflicting with RequireJS. So this is technically not an issue on esbuild or Web Chat side, but globalize-runtime, parse-srcset vs. RequireJS.

Specific Changes

  • createPrecompiledGlobalize will emit bundled PrecompiledGlobalize.js
  • botframework-webchat-api will import from the bundled PrecompileGlobalize.bundle.js
  • I have added tests and executed them locally
  • I have updated CHANGELOG.md
  • I have updated documentation

Review Checklist

This section is for contributors to review your work.

  • Accessibility reviewed (tab order, content readability, alt text, color contrast)
  • Browser and platform compatibilities reviewed
  • CSS styles reviewed (minimal rules, no z-index)
  • Documents reviewed (docs, samples, live demo)
  • Internationalization reviewed (strings, unit formatting)
  • package.json and package-lock.json reviewed
  • Security reviewed (no data URIs, check for nonce leak)
  • Tests reviewed (coverage, legitimacy)

@compulim compulim changed the title Bundle globalize-runtime Disable AMD in our bundle and bundle globalize-runtime May 15, 2025
@compulim compulim changed the title Disable AMD in our bundle and bundle globalize-runtime Disable AMD in our bundle May 15, 2025
@compulim compulim marked this pull request as ready for review May 15, 2025 19:17
@OEvgeny
Copy link
Copy Markdown
Collaborator

OEvgeny commented May 15, 2025

Given the quote from AMD wiki:

AMD can be used as a transport format for CommonJS modules as long as the CommonJS module does not use computed, synchronous require('') calls. CommonJS code that use computed synchronous require('') code can be converted to use the callback-style require supported in most AMD loaders.

Why don't we transpile AMD -> CJS? We don't need the transport part.

@compulim
Copy link
Copy Markdown
Contributor Author

Given the quote from AMD wiki:

AMD can be used as a transport format for CommonJS modules as long as the CommonJS module does not use computed, synchronous require('') calls. CommonJS code that use computed synchronous require('') code can be converted to use the callback-style require supported in most AMD loaders.

Why don't we transpile AMD -> CJS? We don't need the transport part.

It is because esbuild don't understand nor want to touch the ancient AMD. 🤣

Most old packages are UMD (AMD fallback to CJS), so even esbuild don't support AMD, it's fine.

The only case that is not working for esbuild are packages that are AMD only... not UMD, IIFE or CJS. This is super rare nowaday.

@compulim compulim enabled auto-merge (squash) May 15, 2025 19:33
@compulim
Copy link
Copy Markdown
Contributor Author

Also on another side... our bundle/IIFE, we do transpilation and this can be arguably done (AMD -> CJS/ESM).

But for CJS/ESM, we do nothing related to bundling, so web devs will need to do this tranpsilation themselves in their bundling process.

@OEvgeny
Copy link
Copy Markdown
Collaborator

OEvgeny commented May 15, 2025

It is because esbuild don't understand nor want to touch the ancient AMD. 🤣

However, globalize-runtime, parse-srcset, etc. use anonymous modules in AMD via UMD. Thus, i

These might be the reasons why people weren't able to migrate to esbuild, so it doesn't have much adoption besides being a crucial piece in widely used bundlers such as Vite.

Right now the scene of native (non-JS written) bundlers is rather saturated with the addition of:

  • rspack
  • Vite (rolldown)

I expect the esbuild usage to decline more in the near future.

For web devs who consume our CJS/ESM, they will need to remove AMD glue code in their bundler. For Web Chat, we emit CJS/ESM code as faithfully as the source code. That means we do minimal transpilation, e.g. no async to generator helper, no rest/spread helpers, etc.

That said I expect these bundlers are configured to overcome such issues, and the only one left seems the esbuild itself.

@compulim were you able to test non-bundled code is useful with the default esbuild configuration? Or we need to provide additional guidance for our users using esbuild?

There still might be an issue for users with RequireJS + custom esbuild for own deps, e.g. in CMS environments etc.

@compulim compulim merged commit 4c7400a into microsoft:main May 15, 2025
25 checks passed
@compulim compulim deleted the fix-requirejs branch May 15, 2025 19:46
@compulim
Copy link
Copy Markdown
Contributor Author

I think for the very long term is that we don't need bundler. We just need bundling service like esm.sh. 🤣

For additional guidance, currently we don't have docs related to how to bundle stuff. And I am unsure it falls into our responsibility on writing this or if people just search the keyword "RequireJS" in Web Chat repo and they will find the solution. I am not sure. 🤷🏻‍♂️

At the end of the day, it's not an issue related to esbuild or Web Chat, it's about RequireJS reached EOL 5 years ago and most package devs don't care if their stuff work with RequireJS or not. If the package devs opted to keep things up-to-date, they should be using CJS and even ESM-only by now and they won't conflict with RequireJS.

@compulim
Copy link
Copy Markdown
Contributor Author

If there is a better choice while keeping our requirements, I would move away from globalize. But currently, there are no competition in that area right now.

For parse-srcset, it's used by sanitize-html, so it's their choice to move away from a 8 year old package into something newer and more maintained.

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.

Issue with Sharepoint Framework and Version >= 4.17.0

2 participants