Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit ESM build via tsup #5148

Merged
merged 83 commits into from
Apr 30, 2024
Merged

Emit ESM build via tsup #5148

merged 83 commits into from
Apr 30, 2024

Conversation

compulim
Copy link
Contributor

@compulim compulim commented Apr 26, 2024

Changelog Entry

Breaking changes

  • Removed deprecated code: connect*, useRenderActivity, useRenderActivityStatus, useRenderAvatar, in PR #5148, by @compulim
  • Added named exports in both CommonJS and ES Modules module format, in PR #5148, by @compulim

Added

  • Added moduleFormat and transpiler build info to <meta> tag, in PR #5148, by @compulim

Fixed

  • Fixes type portability issues by exporting types from all exported code, in PR #5148, by @compulim
  • Fixes missing exports of useNotifications, in PR #5148, by @compulim

Description

We are adding ES Modules (named exports only) along with CommonJS (both named and unnamed exports).

We expect CommonJS will only used by Webpack 4. And for most web developers who are already using modern pipeline (esbuild, Webpack 5, etc.), they should be using ES Modules.

Design

In our previous citation work, we are adding valibot packages. However, this is breaking create-react-app because of CJS/ESM interoperability. To efficiently resolve the issue, we are exporting ESM by esbuild and tsup.

tsup also tells us a lot of type portability issues. We are resolving many of them in this pull request.

Type portability

Some exported code contains types that are not exported. We deployed 2 strategies:

  • For majority, we are exporting types that should be public
  • Otherwise, we recreate part of the types that should be public
    • For example, the whole type of WebChatAPIContext should not be exported, but only partial of it
    • Some WebChatAPIContext is directly exporting dispatchers from our internal Redux, so we are rewriting the type so it don't need to expose the typing of our internal Redux

Unify exporting types

All types are now exported via export { type SomeType }, instead of export type SomeType. Default exports should be reserved for code.

Precompiling globalize

globalize default use CommonJS/AMD for its runtime. This doesn't play well with esbuild. We are replacing its code-emitting template to emit solely CJS instead.

We also tried to emit ESM but it will call require() in some part of globalize. Switching between CJS/ESM is not very foolproof until globalize exports ESM.

Emitting instrumentation code

We cannot use esbuild-plugin-istanbul because it sometimes not emitting instrumentation code related to JSX.

We have to add @babel/preset-typescript and @babel/preset-react before babel-plugin-istanbul.

We looked at @istanbuljs/instrumenter but printing the code seems more difficult to do than simply running @babel/core/transformAsync.

Version of @types/react

It is confusing to use @types/react@18 with [email protected]. We are unifying all versions to be @types/react@16 instead.

Some FC/VFC were removed in favor of newer type signature. Some SomeComponent.propTypes and SomeComponent.defaultProps were also removed.

We should do a bigger push to clean up FC/VFC/propTypes/defaultProps and mark all props as read-only.

Specific Changes

  • Renamed packages/fluent-theme/package.json/main to botframework-webchat-fluent-theme.js from index.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)

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.

3 participants