time is now a number. Cleaned up code #4381
Conversation
| /** | ||
| * replacement for lodash cloneDeep that preserves type. | ||
| */ | ||
| export function cloneDeep<T>(obj: T): T { |
There was a problem hiding this comment.
In Node 17, they've added a native function for this (https://developer.mozilla.org/en-US/docs/Web/API/structuredClone#browser_compatibility). We can't use it yet since we support old Node versions, but can we add a TODO to switch to this once Node 18 is our minimum version?
I think it's ok to make replace _.cloneDeep with parse+stringify for now, but I want to point out that parse+stringify does behave unexpectedly when called on objects that include Dates, Maps, undefined, and other types that don't map cleanly to JSON (https://stackoverflow.com/questions/122102/what-is-the-most-efficient-way-to-deep-clone-an-object-in-javascript).
If you think this is a good enough reason to not use parse+stringify, we could instead switch to depending on lodash.cloneDeep (https://www.npmjs.com/package/lodash.clonedeep).
There was a problem hiding this comment.
Added todo and used actual recursion to support RegExp, Date, and Map.
| * replacement for lodash cloneDeep that preserves type. | ||
| */ | ||
| // TODO: replace with builtin once Node 18 becomes the min version. | ||
| export function cloneDeep<T>(obj: T): T { |
There was a problem hiding this comment.
ah yes I'm definitely going to be using this.
inlined
force-pushed
the
inlined.timeout-seconds
branch
from
9f39375
to
e11505c
Compare
Tester reported a bug where the functions SDK is reporting "timeoutSeconds" (number) from the container contract but we're expecting "timeout" (Duration).
This bug changes the v1beta1.Manifest parsing to expect the correct timeoutSeconds. Since the container contract and GCFv2 use timeoutSeconds, I switched backend.Endpoint to use timeoutSeconds. This required me to look into the emulator code...
In the emulator code it became clear that timeout wasn't clearly typed (code was defensive for either a string or an int). I fixed this to always be a number inside the emulator code, but then had to address the extensions manifest parsing. That was using an
anytype so I added some type annotations. The type annotations require a literal type which meant I had to add annotations to a test and those annotations where getting lost with_.cloneDeep. Since lodash is deprecated I made my owncloneDeepfunction and moved lodash to use it. And I used that opportunity to clean up lodash from the affected files...... Oh... and
expectneeds to be awaited if comparing against a promise or the test will start failing if the promise doesn't resolve within the same tick of the runloop...... I really let this one snowball. Sorry.