botbuilder-js icon indicating copy to clipboard operation
botbuilder-js copied to clipboard

Using botframework-streaming in browser/webpack@5/esbuild does not work

Open compulim opened this issue 4 years ago • 4 comments

Github issues should be used for bugs and feature requests. Use Stack Overflow for general "how-to" questions.

Versions

4.14.1

Describe the bug

The botframework-streaming is not configured/transpiled correctly to use in browser.

To Reproduce

Steps to reproduce the behavior:

  1. Use webpack@5 or esbuild to consume botframework-streaming
  2. Upload a file via DL ASE protocol

Expected behavior

It should able to upload a file.

Instead, due to buffer, process.nextTick(), and stream is not available from webpack@5 and esbuild, the package will throw an exception.

Due to development team structure and policies, customers may not have access to configurations of Webpack or esbuild. For example, in some customers' scenario, they are forbidden to "eject" the app from its scaffold/template. Thus, they would not have access to any configurations. SDK should work as by default and transpiled and bundled correctly.

Thus, the botframework-streaming package must not request the customer to change their project/host configuration to use botframework-streaming.

Screenshots

If applicable, add screenshots to help explain your problem.

Additional context

How to package correctly

Please look at the build scripts of uuid package on how they do packaging. They cover many scenarios including: CJS, ESM, browser-only (crypto vs. Web Crypto). And the techniques they applied, should be sufficient for botframework-streaming package.

Also read about exports field in package.json.

In short:

  • It should not export a huge/bundled JS (i.e. should not export a single file that contains all the code)
    • This enables dependents to optimize their package through tree shaking
    • Bundling should be done by the very last dependent in the whole chain, i.e. done by the web/mobile app
  • Code referenced by package.json/main field should be ES5 and CommonJS (i.e. use require() to import packages)
  • Code referenced by package.json/module field should be ES5 and ES Module (i.e. use import statement)
  • The package.json/browser field should only override certain files that does not work in browser
    • For example, if ./lib/abc.js contains code that only works in Node.js but not browser, the package.json/browser field should redirect to another file, such as, ./lib/abc.browser.js which will work in browser
    • It should not blindly redirect every file. The redirection is only good for scenarios like crypto vs. Web Crypto, ws vs. WebSocket, cross-fetch vs. fetch
  • The package.json/exports['.'].import field should be same as package.json/module field
  • The package.json/exports['.'].require field should be same as package.json/main field
  • If async or other post-ES5 features are used
    • Use @babel/preset-env to transpile to ES5
    • Use @babel/transform-runtime to polyfill core-js@3, it will limit the exposure of the polyfills only to code inside the package
    • Do not leak any polyfills to the environment. E.g. if the environment does not have Promise interface, after loading this JS, window.Promise must continue to be undefined
      • One easiest way to test, is to load the JS in IE11. It should load properly and window.Promise must continue to be undefined

How to test if the packaging is done correctly

Write index.js to consume botframework-streaming

import * as BFSE from 'botframework-streaming';

// Write some test cases that hit certain critical code paths in BFSE, such as file upload.
console.log(BFSE);

Create a new web app project

Create a new empty project root.

Run npm init to initialize the project.

Run npm install botframework-streaming to install the package. (You can also install botframework-streaming-*.tgz for local testing).

Bundle with webpack@5

Install Webpack by running npm install webpack@5.

Create webpack.config.json:

{
  "mode": "production",
  "entry": "index.js"
}

Note: Do not use babel-loader or any loaders to load any JS files. The published package should be ES5 of both Common JS and ES Module format. Transpile, if needed, should be done before bundle.

Then, run webpack. It will produce output at dist/main*.js.

Write a HTML page to load the dist/main*.js file.

Navigate to this page in all supported browsers (including mobile browsers). The code in index.js should run completely and successfully. For example, if the code in index.js contains file uploading logic, running the HTML page should upload a file.

Bundle with esbuild

Install esbuild by running npm install esbuild

Run esbuild --bundle --outfile=dist/main-esbuild.js ./index.js. It will produce an output file at dist/main-esbuild.js.

Write another HTML page to load the dist/main-esbuild.js file.

Navigate to this page in all supported browsers (including mobile browsers). The code in index.js should run completely and successfully.

Test using IE11

IE11 is an excellent tool for testing if the package is bundled/transpiled correctly. Using latest browsers also works, but it is not as trivial as loading the bundle in IE11.

Two criteria:

  • It should load properly in IE11 without errors
    • This will guarantee the package is correctly transpiled to ES5
  • Polyfills must not be leaked to the environment
    • Since IE11 requires a lot of polyfills, it is much easier to detect leakage of polyfills in IE11, than in other browsers

Pre-transpile or post-transpile

Currently, there is a trend of exporting ESnext code. ESnext code may not work in latest browsers.

Devs are slowly moving from pre-transpile to post-transpile.

  • Pre-transpile
    • Run Babel to turn all ESnext into ES5
    • Publish/consume the package
    • Run bundler to bundle all app code into a single file (which should only contain ES5 code)
  • Post-transpile
    • Publish/consume the package
    • Run bundler to bundle all app code into a single file (which could contain ESnext code)
    • Run Babel to transpile the bundle to ES5

For now, it's good to do pre-transpile. For the next 6-18 months, we may move to post-transpile. There are few advantages of post-tranpsile:

  • Smaller bundle because of shared polyfills
  • Faster bundling during development time (no need to Babel to ES5 during development, since web devs are usually using canary browsers, which support ESnext)

compulim avatar Jul 24 '21 09:07 compulim

@stevengum DLJS is having issues when bumping Webpack from 4 to 5 (due to security vulnerability). Starting from webpack@5, the botframework-streaming package is no longer working properly due to poor bundling/transpilation.

compulim avatar Jul 24 '21 09:07 compulim

Assigning to @JuanAr for investigation.

mrivera-ms avatar Mar 15 '22 20:03 mrivera-ms

@mrivera-ms in progress

sw-joelmut avatar Mar 16 '22 17:03 sw-joelmut

Hi @mrivera-ms, @compulim. We were able to reproduce the issue using the DL ASE protocol with the 4.14.1 version of botbuilder, where the bot doesn’t receive the attachment in the Activity when sending a file. Bumping the botbuilder version in the deployed bot to the latest 4.15.0, the bot started receiving the attachment through DL ASE.

Bot sample with the reproduced the issue (with instructions): issue-3884.zip

To be able to test the DL ASE protocol functionality, we followed these guides to configure our environment:

imagen

sw-joelmut avatar Mar 21 '22 17:03 sw-joelmut

Closing. @compulim If you feel differently, please reopen.

tracyboehrer avatar Mar 18 '24 20:03 tracyboehrer