• RussellF (unregistered)

    There are circumstances in which this would make sense, as a way to do a deep copy. Maybe the first instance of this was in a case where that was true.

  • (nodebb) in reply to RussellF

    It seems like an inefficient way for doing a deep copy though.

  • Robin (unregistered)

    Yeah, stringify then parse is a fairly common way to deep copy objects, provided they contain just data (no functions etc) - which is what you get from most API responses. It's unlikely the copy is actually necessary, but I guess it was in one place and spread across the codebase in a "this worked once, it's not my job to think" kind of way.

    But there's so much else even in this short snippet that upsets me. Using Object.keys to iterate through the keys and use those to get the values - hello, Object.values is also a thing, as is Object.entries (which is also used here so it's not like they were ignorant of it) if you need both keys and values.

    And worst of all (for me at least), using map and not doing anything with the returned array - forEach is the correct method to use to simply iterate through an array to do something (or just an old-fashioned for loop, come to that), and "but it works/passes the tests" (which it will) is not a good reason to confuse readers (and degrade performance, although that's very unlikely to matter) by using the wrong method...

  • Robin (unregistered)

    Actually, given the unnecessary map is happening in a loop, and we're told there's a UI performance problem, maybe the data is huge and this is one time constructing arrays unnecessarily does have a noticeable performance impact!

    Although my money would still be on the main culprit being something idiotic inside nested loop that we've not been shown.

  • Dennis (unregistered)

    JSON.parse(JSON.stringify()) is really the only widely available and standardised non-homegrown deep copy method. Hell, it's right there in the MozDev documentation for Object.assign: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign#warning_for_deep_clone

  • Dennis (unregistered) in reply to Dennis

    (okay fair, without installing some third party library)

  • (nodebb)

    More fun if it was map(([value, key]) .... I hit that a few years ago....

  • (nodebb) in reply to Dennis

    @Dennis Would structuredClone be an alternative? https://developer.mozilla.org/en-US/docs/Glossary/Deep_copy Though it is mentioned as not strictly portable, it seems like the distinction between "JavaScript itself" and "platforms that implement JavaScript" might be moot.

  • Yikes (unregistered)

    It seems like an "everyone gets their own copy" approach, which probably exists because the "everyone is used to trashing the objects" culture is prevalent.

  • Dennis (unregistered) in reply to R3D3

    It will be... When it has had a few more revisions in the biggest browsers. Always some laggard that will blame us that s/he didn't update their browser.

    Once it hits maybe a year in all major browsers (It was implemented in Firefox in 2021, Chrome/Edge in February and Safari in March), it should be safe to use without a fallback. At least safe enough to tell lagging customers to get their act together.

  • (nodebb) in reply to R3D3

    I was gonna say, it's not moot if your code might be run in both front and back end; but it looks like even Node and Deno implement it, so yeah, it does seem kind of a subtle distinction. Like, technically, a client doesn't have to implement it; but they all do, so ¯\\_(ツ)_/¯

    Addendum 2022-07-11 15:33: Curses! One too many backslashes.

  • (nodebb)

    See my experience at Confessions of a Deep Copy.

    Really, they're being paid to write this, and they write like a girl who had just installed nodejs three days prior?

    Also I'm pretty shocked that this is actually on MDN.

Leave a comment on “Stuttering Strings”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article