Add useEmulator() to Storage #4346
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hsubox76
requested review from
Feiyang1,
hiranya911 and
schmidt-sebastian
as code owners
🦋 Changeset detectedLatest commit: ed28cf6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Size Analysis Report |
Feiyang1
approved these changes
Feb 3, 2021
Binary Size ReportAffected SDKs
Test Logs |
hsubox76
changed the title
hsubox76
force-pushed
the
ch-storage-emulator
branch
from
4f740a3 to
b6e0ffd
Compare
hsubox76
commented
Feb 22, 2021
| export function makeUrl(urlPart: string): string { | ||
| return `https://${DEFAULT_HOST}/v0${urlPart}`; | ||
| export function makeUrl(urlPart: string, host: string): string { | ||
| const protocolMatch = host.match(/^(\w+):\/\/.+/); |
There was a problem hiding this comment.
Added this because emulator uses http when production use defaults to https. Seems cleaner to let the host string contain a protocol or not, and if so, parse it in this one function, than to store protocol as a separate property on StorageService and get it everywhere service.host is also used.
Changeset File Check ✅
|
NothingEverHappens
approved these changes
Mar 31, 2021
NothingEverHappens
approved these changes
Mar 31, 2021
| export function makeUrl(urlPart: string): string { | ||
| return `https://${DEFAULT_HOST}/v0${urlPart}`; | ||
| export function makeUrl(urlPart: string, host: string): string { | ||
| const protocolMatch = host.match(/^(\w+):\/\/.+/); |
There was a problem hiding this comment.
nit/optional, I think using replace with callback would save some lines and be more readable
There was a problem hiding this comment.
I couldn't figure out how to easily set up a regex that would call the callback if the protocol wasn't there so I left it alone for now. We can always optimize any time so let me know if you have a snippet you want to put in.
hsubox76
changed the title
|
@hsubox76 is this available for use yet? From the docs it seems we need to download/reference |
|
It's merged and should go out in the release next Thursday. |
|
Thanks! |
Merged
|
Is this feature actually ready for firebase developers? I ask because as of v9.10.0 the firebase-tools CLI still does not include storage in the list of emulators that can be initialised. |
|
@vbeffa thank you! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add
storage().useEmulator()to current storage package anduseStorageEmulator()to storage-exp.Wait for emulator to be available before merging.