4

I have a fairly deep interface declared that looks something like this:

export interface Job {
    JobId: JobId; // type JobId = string
    UserId: UserId; // type UserId = string
    JobName: string;
    AudioFile: JobAudioFile; // this is an interface
    Status: JobStatus; // this is an enum
    Tracks: JobTracks[]; // 'JobTracks' is an enum
    Results: JobResults; // this is an interface
    Timestamps: JobTimestamps // interface
  }

Most of the members of this interface are themselves interfaces, with the general architecture following this pattern of using enums, strings, arrays and more interfaces. All code is written as TypeScript, transpiled down to JS and uploaded to AWS as JS. (Node 8.10 is running on AWS)

At one point in the code, I need to make a deep copy of a Job instantiation which was passed in as a function parameter:

export const StartPipeline: Handler = async (
  event: PipelineEvent
): Promise<PipelineEvent> => {
  console.log('StartPipeline Event: %o', event);

  const newBucket = await copyToJobsBucket$(event.Job);
  await deleteFromOriginalBucket$(event.Job);

  console.log(`Job [${event.Job.JobId}] moved to Jobs bucket: ${newBucket}`);

  event.Job.AudioFile.Bucket = newBucket;
  event.Job.Status = Types.JobStatus.Processing;

  // update the job status

  // VVV PROBLEM OCCURS HERE VVV
  const msg: Types.JobUpdatedMessage = new Types.JobUpdatedMessage({ Job: Object.assign({}, event.Job) }); 
  await Send.to$(event.Job.UserId, msg);

  return { ...event };
};

The definition of the JobUpdatedMessage:

  export class JobUpdatedMessage extends BaseMessage {
    constructor(payload: { Job: Types.Job }) {
      console.log('Incoming: %o', payload);
      const copy: object = { ...payload.Job };

      // VVV PROBLEM ON NEXT LINE VVV
      const filtered = JobUtils.FilterJobProperties(copy as Types.Job);

      super(MessageTypes.JobUpdated, filtered);
    }
  }

The problem is after the call to JobUtils.FilterJobProperties, payload.Job has also been mutated in an undesirable and unexpected way.

Here's the implementation of JobUtils.FilterJobProperties:

export const FilterJobProperties = (from: Types.Job): Types.Job => {
    const fieldsToRemove: string[] = [
      'Transcripts.GSTT',
      'Transcripts.WSTT',
      'Transcripts.ASTT',
      'TranscriptTracks',
      'Transcripts.Stream.File',
      'Transcripts.Stream.State',
      'AudioFile.Bucket',
      'AudioFile.S3Key',
    ];

    let job: Types.Job = { ...from }; // LINE ONE

    fieldsToRemove.forEach(field => _.unset(job, field));  // LINE TWO

    return job;
  };

(I'm using the lodash library here)

The line market 'LINE TWO' is also mutating the from function parameter, even though on 'LINE ONE' I'm doing what I think is a deep clone of from.

I know that this is the case because if I change 'LINE ONE' to:

// super hard core deep cloning
let job: Types.Job = JSON.parse(JSON.stringify(from));

... everything works as expected. from is not mutated, the resulting JobUpdatedMessage is as expected, and StartPipeline's event parameter doesn't have a bunch of properties removed from event.Job.

I struggled with hours on this, including relearning everything I believed I knew about cloning objects in Es6 using the spread operator.

Why was 'LINE ONE' mutating the input as well?

John Dibling
  • 99,718
  • 31
  • 186
  • 324
  • [`...` is not an operator!](https://stackoverflow.com/questions/37151966/what-is-spreadelement-in-ecmascript-documentation-is-it-the-same-as-spread-oper/37152508#37152508) – Felix Kling Feb 05 '19 at 20:41

1 Answers1

9

Spread operator does shallow cloning same as Object.assign()

Shallow-cloning (excluding prototype) or merging of objects is now possible using a shorter syntax than Object.assign().

Spread operator

An example to understand spread operator and shallow cloning.

let obj = { 'a': { 'b' : 1 },'c': 2}

let copy = {...obj}

copy.c = 'changes only in copy'  //shallow-cloned 
copy.a.b = 'changed'             // still reference

console.log('original\n',obj)
console.log('\ncopy',copy)

Using spread operator object is shallow cloned so all the first level properties will become a copy while all the deeper level properties will still remain the references.

so as you see in example c property doesn't affect the original object since it is one first level depth, on the other hand b property changes affect the parent properties because it is at deep level and is still reference.

Code Maniac
  • 37,143
  • 5
  • 39
  • 60
  • That makes sense. Why is that not the actual behavior I see here when I run Node on the command line? `> a = { foo: '1', bar: { baz: 42, bat: { deep: 'hi' } } } { foo: '1', bar: { baz: 42, bat: { deep: 'hi' } } } > b = {...a} { foo: '1', bar: { baz: 42, bat: { deep: 'hi' } } }` – John Dibling Feb 05 '19 at 20:27
  • @john what did you expect? – Jonas Wilms Feb 05 '19 at 20:28
  • 2
    @JohnDibling—a shallow clone will *look* just like a deep one, until you change a deep property. I.e. you can't see the difference between a value and a reference where they both resolve to the same value. – RobG Feb 05 '19 at 20:33
  • Hey, @JonasWilms. I've been struggling to answer that question. All I can say is 'not that.' However this is all making sense, and I think it also helps to explain some odd behavior I saw in my frontend which uses NGRX - also full of `{...obj}` copies. – John Dibling Feb 05 '19 at 20:37
  • @JohnDibling i have added an example with more explanation you can see. i hope it will clear things more – Code Maniac Feb 05 '19 at 20:40
  • [`...` is not an operator!](https://stackoverflow.com/questions/37151966/what-is-spreadelement-in-ecmascript-documentation-is-it-the-same-as-spread-oper/37152508#37152508) – Felix Kling Feb 05 '19 at 20:40
  • @CodeManiac: It did, a great deal. Thanks for the edit. You know, I can't help but feel that in some ways C++ is so much simpler than JavaScript. References and pointers are right on the surface there, so it's real clear when I just wrote a piece of code that's wrong. The devil you know, I suppose. – John Dibling Feb 05 '19 at 20:42
  • @FelixKling: Interesting. Years ago in the C++ tag people used to get bent out of shape when people referred to "The STL." The library that began life as the Standard Template Library was later made a part of the formal language spec itself, thus becoming simply "the C++ standard library." When people got grumpy about that I found myself thinking, "Whatever. I knew what they meant. It doesnt change how I write code." With the construct I was referring to in my post, does the fact that it's not an operator change how I should write code? – John Dibling Feb 05 '19 at 20:49
  • @John: It's all right if you know what is meant. We could also call it "potato snake". But not everybody understands what it means or they make incorrect deductions from the name, which leads to [questions like this one](https://stackoverflow.com/q/35019557/218196). Ultimately, terminology is important in communication and the term "operator" has a very specific meaning in JavaScript (and programming languages in general). I believe using misleading terminology makes it more difficult for less knowledgeable people to understand what's going on. – Felix Kling Feb 05 '19 at 21:15
  • @John: Either way, I'm just pointing it out. Do whatever you want :) – Felix Kling Feb 05 '19 at 21:15
  • @FelixKling: Not trying to pick a fight about pedantry or semantics. My little story may not have given this impression, but my question was meant in good faith. It seems what you're saying is that since `...` isn't an operator, `...` can't be 'called' in the way a function can be called. Yes? (BTW, I actually did not realize that `...`isn't an operator, so thanks for the knowledge.) – John Dibling Feb 06 '19 at 21:34
  • @JohnDibling: Right. An operator takes one ore more operands and produces a value as a result. But `...` doesn't produce a value (hence not an operator), it's simply part of the array literal/function call/function definition/etc syntax. – Felix Kling Feb 06 '19 at 22:30