Skip to content

Fix port conflicts with functions:shell #3223

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

Merged
merged 6 commits into from
Mar 23, 2021
Merged

Fix port conflicts with functions:shell #3223

merged 6 commits into from
Mar 23, 2021

Conversation

samtstern
Copy link
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
Copy link
Contributor

@joehan joehan left a comment

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.

var initializeContext = function (context) {
_.forEach(emulator.triggers, function (trigger) {
const initializeContext = (context: any) => {
_.forEach(emulator.triggers, (trigger) => {
Copy link
Contributor

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
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