Skip to content

Fix port conflicts with functions:shell#3223

Merged
samtstern merged 6 commits into
masterfrom
ss-fix-3210
Mar 23, 2021
Merged

Fix port conflicts with functions:shell#3223
samtstern merged 6 commits into
masterfrom
ss-fix-3210

Conversation

@samtstern

Copy link
Copy Markdown
Contributor

Description

Fixes #3210

Also I converted the file to .ts while I was in there

Scenarios Tested

  • Picking up the port from firebase.json
  • Detecting another running emulator and printing a warning
  • Using the --port flag

Sample Commands

N/A

@google-cla google-cla Bot added the cla: yes Manual indication that this has passed CLA. label Mar 19, 2021
@samtstern samtstern requested a review from joehan March 19, 2021 16:26

@joehan joehan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM after a few tiny cleanups + lodash removal.

Comment thread src/functionsShellCommandAction.ts Outdated
Comment thread src/functionsShellCommandAction.ts Outdated
Comment thread src/functionsShellCommandAction.ts Outdated
var initializeContext = function (context) {
_.forEach(emulator.triggers, function (trigger) {
const initializeContext = (context: any) => {
_.forEach(emulator.triggers, (trigger) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Optional - it would be pretty easy to refactor this chunk to use plain JS instead of lodash, and then we're one step closer to removing the dependency for good

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I got rid of _.forEach and _.includes but _.set with dot-separated path notation is a bit tricky so I left that. We'll probably need to create our own utils.set to replace that throughout the codebase and that seems out of scope for this PR.

@samtstern samtstern merged commit 20e11f8 into master Mar 23, 2021
@bkendall bkendall deleted the ss-fix-3210 branch August 4, 2021 19:27
devpeerapong pushed a commit to devpeerapong/firebase-tools that referenced this pull request Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Manual indication that this has passed CLA.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't connect to functions shell when hosting emulator is running

2 participants