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

Cannot bump typescript to 5.6 due to botbuilder INodeBuffer error #4746

Closed
corinagum opened this issue Sep 11, 2024 · 5 comments · Fixed by #4757
Closed

Cannot bump typescript to 5.6 due to botbuilder INodeBuffer error #4746

corinagum opened this issue Sep 11, 2024 · 5 comments · Fixed by #4757
Assignees
Labels
feature-request A request for new functionality or an enhancement to an existing one.

Comments

@corinagum
Copy link
Collaborator

corinagum commented Sep 11, 2024

Versions

Botbuilder-js: 4.23.x
Node: 18.20.4
Browser: N/A
OS: Mac 14.6.x

Describe the bug

When bumping Typescript to 5.6.x in the Teams AI library, typescript errors caught in the latest TS update causes the build to fail due to incorrect extend of Uint8Array. Error message:

 | ../../node_modules/botframework-streaming/lib/interfaces/INodeBuffer.d.ts(15,18): error TS2430: Interface 'INodeBuffer' incorrectly extends interface 'Uint8Array'.
 |   The types returned by 'slice(...)' are incompatible between these types.
 |     Type 'this' is not assignable to type 'Uint8Array'.
 |       Type 'INodeBuffer' is not assignable to type 'Uint8Array'.
 |         The types returned by 'entries()' are incompatible between these types.
 |           Type 'IterableIterator<[number, number]>' is missing the following properties from type 'ArrayIterator<[number, number]>': map, filter, take, drop, and 9 more.

To Reproduce

Steps to reproduce the behavior:

  1. Open any Botbuilder TS sample
  2. In package.json, upgrade typescript to 5.6.x
  3. (If applicable) Remove node_modules/ lock files before building the sample
  4. Install && build the sample
  5. See error in console
@corinagum corinagum added bug Indicates an unexpected problem or an unintended behavior. needs-triage The issue has just been created and it has not been reviewed by the team. labels Sep 11, 2024
corinagum added a commit to microsoft/teams-ai that referenced this issue Sep 13, 2024
…#2014)

## Linked issues

closes: #1947
closes #1371

#### TLDR: Node 20 is now supported!!! If you're eager for this, you
shouldn't need to wait for a release, as the affected packages were our
samples.

## Details

- Modified package configuration for `typescript` to only allow patch
updates. See Change details below for more information. This is
(ideally) temporary.
- Remove `@microsoft/teamsfx` as a dependency - it turns out this
dependency is entirely unused, but due to sub-dependencies was
preventing us from being able to support Node >=18. Because of this and
`[email protected]` now supporting node 20, teams-ai js package now
also runs with node 20 too!
- Removed `browserify` - unused dependency
- Updated Powered by AI docs to slightly clarify how AI module may use a
`planner`.

#### Change details

[TS made an
update](https://devblogs.microsoft.com/typescript/announcing-typescript-5-6)
that in 5.6 reveals previously 'hidden' TS errors.
`gpt-tokenizer` and `botbuilder` have TS errors that 5.6 catches. I
filed bugs for both packages:
 
[[Bug] Cannot upgrade to TS 5.6x due to build errors · Issue #49 ·
niieani/gpt-tokenizer
(github.com)](niieani/gpt-tokenizer#49)
[Cannot bump typescript to 5.6 due to botbuilder `INodeBuffer` error ·
Issue #4746 · microsoft/botbuilder-js
(github.com)](microsoft/botbuilder-js#4746)

## Attestation Checklist

- [x] My code follows the style guidelines of this project

- I have checked for/fixed spelling, linting, and other errors
- I have commented my code for clarity
- I have made corresponding changes to the documentation (updating the
doc strings in the code is sufficient)
- My changes generate no new warnings
- I have added tests that validates my changes, and provides sufficient
test coverage. I have tested with:
  - Local testing
  - E2E testing in Teams
- New and existing unit tests pass locally with my changes

### Additional information

> Feel free to add other relevant information below

---------

Co-authored-by: Corina Gum <>
@tracyboehrer
Copy link
Member

@ceciliaavila Can you comment on this? @corinagum This gets touchy since changing TS version can break people, no?

@ceciliaavila
Copy link
Collaborator

@ceciliaavila Can you comment on this? @corinagum This gets touchy since changing TS version can break people, no?

@tracyboehrer, we are currently supporting TS versions from 4.6 to 5.5. Adding support for 5.6 may require some work.

@corinagum
Copy link
Collaborator Author

@tracyboehrer Agreed it is touchy, but I still think we would want to support it.

@tracyboehrer
Copy link
Member

@corinagum You aren't wrong. We're going to look into it.

@tracyboehrer tracyboehrer added feature-request A request for new functionality or an enhancement to an existing one. and removed needs-triage The issue has just been created and it has not been reviewed by the team. bug Indicates an unexpected problem or an unintended behavior. labels Sep 23, 2024
@sw-joelmut
Copy link
Collaborator

Hi everyone,

We were able to reproduce the issue using TypeScript 5.6.2 with the ESNext configuration in the tsconfig.json.
image

By replacing ESNext with targets prior to that (e.g. ES2023), the issue doesn’t happen.
It seems that there is a problem with the TS 5.6 and ESNext combination, because previous versions of TS are working fine.
We are working on fixing the errors thrown when using this configuration (TS 5.6 + ESNext target), but, as a workaround, you can change the target to ES2023.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A request for new functionality or an enhancement to an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants