phetsims / chipper

Tools for developing and building PhET interactive simulations.
MIT License
11 stars 14 forks source link

Separate namespace.register and "inherit" calls from export default. #848

Closed samreid closed 4 years ago

samreid commented 4 years ago

From #846 @pixelzoom pointed out we should separate the namespace.register from the export default.

samreid commented 4 years ago

Ideally we would do the same with inherit statements, but we IDE support for that may be limited anyways because inherit is our own invention.

samreid commented 4 years ago

I wonder if it may be easier to do this in master and guard against it with a lint rule. I also noticed several cases where variables were being created inline like so:

  return areaModelCommon.register( 'AreaModelCommonConstants', {

    // {PhetFont} All fonts
    FACTORS_TERM_FONT: new PhetFont( 36 ), // Terms/numbers in the factors box
    FACTORS_PAREN_FONT: new PhetFont( 40 ), // Parentheses in the factors box
    CALCULATION_X_FONT: new PhetFont( 16 ),
samreid commented 4 years ago

This seems good to do before the migration.

samreid commented 4 years ago

@jonathanolson and I discussed this and determined it is not essential to do before the migrate. However, the namespace.register calls may be convenient to add as a migration rule if there is time. inherit will probably be a separate step.

samreid commented 4 years ago

As a middle ground, let's convert usages that are single-line statements.

samreid commented 4 years ago

I added migration for single-line import/namespace statements and tested a full aqua and lint-everything. Demoting priority until after the migrate to handle multiline statements.

chrisklus commented 4 years ago

It looks like there's many multiline cases (~1160) that will need to be addressed after the migration. @samreid is going to investigate nested macros to handle these.

zepumph commented 4 years ago

Thank you for doing this! I'm afraid there are a few cases here that I am directly responsible for.

samreid commented 4 years ago

I finished converting these and added lint rules to prevent their recurrence. Closing.