add _.extend to bad sim text?

[zepumph]

phetsims/phet-core#71

[zepumph]

We should decide if this is really forbidden in sim text, marking for dev meeting.

[pixelzoom]

When I have full control over the args, and I know that _.extend is appropriate, I'd like to be able to use _.extend. It's more convenient, because unlike merge, I don't need a require statement. For example:

// options shared by all Rectangle instances
const RECTANGLE_OPTIONS = { 
  stroke: 'black',
  lineWidth: 2,
  cornerRadius: 3
};

const redRectangle = new Rectangle( ..., _.extend( { fill: 'red' }, RECTANGLE_OPTIONS );
const greenRectangle = new Rectangle( ..., _.extend( { fill: 'green' }, RECTANGLE_OPTIONS );

[zepumph]

I think that is reasonable, but I also see value in having an established pattern. If everything is the same, it may be a bit easier for the project as a whole.

I also think that there is a point to be made about maintainability. In the example above, it is currently harmless, but perhaps in the future Rectangle will add in a nested set of options, like paintOptions or something. In which case this potentially becomes a harder refactor.

Again I don't feel strongly about this.

[pixelzoom]

perhaps in the future Rectangle will add in a nested set of options, like paintOptions or something. In which case this potentially becomes a harder refactor.

It doesn't become any harder (and in fact doesn't change) because I have full control over all of the args. If my args are unchanged, then it's irrelevant whether Rectangle takes nested options or not, I need to change nothing. If I'm not using nested options, I can use _.extend. If I need to change my args to nested options in the future, then I know that _.extend in inappropriate, and I change to merge. Yes, I need to know that I must use merge. But PhET developers should be familiar with lodash, and this is minor compared to the other types of trouble that you can get into with Javascript.

[samreid]

It's more convenient, because unlike merge, I don't need a require statement.

We could change merge to a preload.

[jonathanolson]

We could change merge to a preload.

I'm seeing a common trend where we find something of general use in phet-core (i.e. most of phet-core) and it's "I wish we didn't have to use a require statement" and "I wish I could use this in other preloads, but it's in require.js".

Should we consider moving some core code into a preload that is outside of the require.js system?

[samreid]

I'm up for making it easy for the entire team, but thought I should mention it doesn't bother me to add the require statement. I use shift-option-cmd-R to automatically add require statements and it has been working well for my situation.

[zepumph]

We will go with adding [Op]ptions = _.extend and [cC]onfig = _.extend to bad SIM text.

[zepumph]

Lint rules added above. Only a few had crept in since the conversion. I'm glad to have added these now. Closing.