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

feat: Update versions for generators #4796

Merged
merged 5 commits into from
Nov 22, 2024

Conversation

ceciliaavila
Copy link
Collaborator

#minor

Description

This PR includes the generators' templates to be updated by the update-versions script by including them in the workspaces list.
It also updates the versions in the package.json file to match the one used in the libraries (4.1.6) and upgrades some dependencies in the TS bots to fix errors found when building the generated bots.

Specific Changes

  • Changed botbuilder dependencies' version to 4.1.6 in all bots' templates.
  • Updated TS bots by adding the types/node package and upgrading typescript dependency to version 4.9.3. Also, updated tsconfig files updating the module and adding moduleResolution and esModuleInterop properties.
  • Added the 'generators/generator-botbuilder/generators/app/templates/' path to workspaces in the package.json file.
  • Updated updateVersions, workspace, and package files to include the new projects in the script.

Testing

These images show the updated version in the bots and the bots working with the changes.
image
image

@ceciliaavila ceciliaavila requested a review from a team as a code owner November 19, 2024 16:54
@coveralls
Copy link

coveralls commented Nov 19, 2024

Pull Request Test Coverage Report for Build 11958877849

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 84.704%

Totals Coverage Status
Change from base Build 11687414604: 0.0%
Covered Lines: 20515
Relevant Lines: 23064

💛 - Coveralls

Copy link
Member

@rido-min rido-min left a comment

Choose a reason for hiding this comment

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

clarify which versions we want to target from the templates
discuss node versions support

@@ -3,13 +3,15 @@
"composite": true,
"declaration": true,
"target": "es2017",
"module": "commonjs",
"module": "NodeNext",
Copy link
Member

Choose a reason for hiding this comment

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

what's the motivation to diverge from commonjs? afaik we have not finalized support to latest node

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We had to include the moduleResolution property to solve a TS issue. It was recommended to match the value for the module property with the one for moduleResolution, however, it works if we keep the commonjs setting.
image

Copy link
Member

Choose a reason for hiding this comment

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

I'd recommend to keep using commonjs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, I was confused, the value for moduleResolution must match the one for module. it does not work with commonjs, but NodeNext supports both CommonJS and ECMAScript modules (documentation).

@tracyboehrer
Copy link
Member

@rido-min This purpose was to support creating and versioning the generators package during build. This is to allow the generators to be built and deployed whenever we do a release of the JS SDK. This does not encompass template specific issues, though we could also make changes there if needed.

@tracyboehrer tracyboehrer merged commit ff562bc into main Nov 22, 2024
13 checks passed
@tracyboehrer tracyboehrer deleted the southworks/update/generators-version branch November 22, 2024 14:40
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.

4 participants