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.

Sign up for GitHub

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

Jump to bottom

fix: fix issue with addWebRoute() api extension method #2990

Merged
merged 5 commits into from
May 11, 2020
Merged

fix: fix issue with addWebRoute() api extension method #2990

merged 5 commits into from
May 11, 2020

Conversation

benbrown
Copy link
Contributor

@benbrown benbrown commented May 11, 2020 •
edited by cwhitten
Loading

Description

This fixes a regression introduced a while ago but discovered today by a user that broke the addWebRoute method.

The method needed to be bound back to the Express object to function correctly.

closes #2990

@benbrown benbrown changed the base branch from stable to master May 11, 2020 17:36
@github-actions
Copy link

Coverage Status

Coverage remained the same at 0.0% when pulling 7e92cda on benbrown/fixwebplugins into f67fcab on master.

a-b-r-o-w-n
a-b-r-o-w-n requested changes May 11, 2020
View reviewed changes
Copy link
Contributor

@a-b-r-o-w-n a-b-r-o-w-n left a comment

Choose a reason for hiding this comment

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

Can you elaborate on the requirement for this change? I'd just like to understand what was breaking.

Composer/packages/extensions/plugin-loader/src/composerPluginRegistration.ts Outdated Show resolved Hide resolved
@benbrown @a-b-r-o-w-n
Update Composer/packages/extensions/plugin-loader/src/composerPluginR…
0dea986 
…egistration.ts

Co-authored-by: Andy Brown <asbrown002@gmail.com>
@benbrown
Copy link
Contributor Author

benbrown commented May 11, 2020 •
edited
Loading

It was throwing an exception because Express wasn't bound to the method anymore, so it was unable to find certain internal methods.

Without this, using the addWebAPI method throws this error:

TypeError: Cannot read property 'lazyrouter' of undefined

This turns out to be the source problem:
expressjs/express#3855

@benbrown benbrown requested a review from a-b-r-o-w-n May 11, 2020 20:12
@cwhitten cwhitten merged commit 3ac5786 into master May 11, 2020
@cwhitten cwhitten deleted the benbrown/fixwebplugins branch May 11, 2020 22:42
@a-b-r-o-w-n a-b-r-o-w-n mentioned this pull request May 15, 2020
Merged
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
@benbrown @a-b-r-o-w-n @cwhitten
fix: fix issue with addWebRoute() api extension method (microsoft#2990)
2e4075f 
* Fix regression that breaks the api.addWebRoute method

* Update Composer/packages/extensions/plugin-loader/src/composerPluginRegistration.ts

Co-authored-by: Andy Brown <asbrown002@gmail.com>

Co-authored-by: Andy Brown <asbrown002@gmail.com>
Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@a-b-r-o-w-n a-b-r-o-w-n a-b-r-o-w-n approved these changes

@boydc2014 boydc2014 Awaiting requested review from boydc2014 boydc2014 is a code owner

@corinagum corinagum Awaiting requested review from corinagum

@cwhitten cwhitten Awaiting requested review from cwhitten cwhitten is a code owner

@luhan2017 luhan2017 Awaiting requested review from luhan2017

@yeze322 yeze322 Awaiting requested review from yeze322

@beyackle beyackle Awaiting requested review from beyackle

@srinaath srinaath Awaiting requested review from srinaath srinaath is a code owner

@tonyanziano tonyanziano Awaiting requested review from tonyanziano

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@benbrown @a-b-r-o-w-n @cwhitten