Skip to content

Enable strictNullChecks for scripts/emulator-testing/ #2066

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 1 commit into from
Aug 12, 2019

Conversation

mikelehen
Copy link
Contributor

I took the emulator-related part out of https://github.com/firebase/firebase-js-sdk/pull/1941/files and reworked it a bit (trying not to use ! assertions). I'll work on all the Firestore stuff next (sorry for being so slow).

NOTE: I left out the stdio: 'inherit' lines you changed to stdio: ['inherit', process.stdout, process.stderr] as I couldn't figure out what that was for. Let me know if I should pull that in for some reason.

Copy link
Contributor

@hsubox76 hsubox76 left a comment

Choose a reason for hiding this comment

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

Thanks! The stdio change was because a certain amount of output from child processes, including some errors, was getting swallowed unless I specified they should write to process.stdout and process.stderr specifically. I'm still not sure exactly how it works and why inherit wouldn't work for those streams but that seemed to be the combination that was getting results.

@mikelehen mikelehen merged commit a20dbea into master Aug 12, 2019
@mikelehen
Copy link
Contributor Author

Interesting... I merged this as-is, but if you remember how to reproduce the issue with child process output getting lost, I can take a peek and reinstate your change if necessary. Else, we can leave this as-is and revisit if it comes up again.

@firebase firebase locked and limited conversation to collaborators Oct 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants