Replace all phet.chipper.getQueryParameter usages with QueryStringMachine

[samreid]

For instance:

test-sims.js has:

var task = phet.chipper.getQueryParameter( 'testTask' ) || 'none';

this would be replaced with:

var task = QueryStringMachine.get( 'testTask',{type:'string',defaultValue:'none'} );

[samreid]

A significant problem is for flags like ?useFeature=false where "false" is being interpreted as truthy.

@zepumph suggested raising this for developer meeting, we may proceed by having each developer take some of the work.

[samreid]

@andrewadare suggested each developer should fix their own cases, because they will be better able to test the changes (because they are more familiar with their own codebases).

[pixelzoom]

Done for all of my sims.

Questions:

(1) It would be nice to have a way to get state of the dev query parameter without having to repeat dev: { type: 'flag' }. Perhaps we need JoistQueryParameters?

(2) To enable logging, some sims check phet.chipper.getQueryParameter( 'log' ) in their namespace file. See for example functionBuilder.js. FBQueryParameters can't be loaded here because that would require functionBuilder.js to be namespaced, and it can't namespace itself. How (or should) we address this?

(3) Who will handle this replacement for common code repositories?

[samreid]

FBQueryParameters can't be loaded here because that would require functionBuilder.js to be namespaced, and it can't namespace itself.

I'm confused about the logic here. It seems like the true problem is that functionBuilder.js must be loaded before FBQueryParameters is loaded, so that namespace be used in FBQueryParameters. Hence it would introduce a cyclic dependency.

[pixelzoom]

... cyclic dependency

Right.

[jonathanolson]

It also looks like initialize-globals now depends on QueryStringMachine? I'm fine taking care of usages I'm familiar with.

[pixelzoom]

To address this issue:

(2) To enable logging, some sims check phet.chipper.getQueryParameter( 'log' ) in their namespace file. See for example functionBuilder.js. FBQueryParameters can't be loaded here because that would require functionBuilder.js to be namespaced, and it can't namespace itself. How (or should) we address this?

I moved the 'log' query parameter from the namespace file (e.g. functionBuilder.js):

  // enables logging output to the console
  if ( phet.chipper.getQueryParameter( 'log' ) ) {
    console.log( 'enabling log' );
    functionBuilder.log = function( message ) {
      console.log( '%clog: ' + message, 'color: #009900' ); // green
    };
  }

... to the query parameters file (e.g. FBQueryParameters):

    // enables console logging
    log: { type: 'flag' }

Usages change from this:

functionBuilder.log && functionBuilder.log( message );

... to this:

FBQueryParameters.log && console.log( message );

The upside is that all query parameters are now documented and processed in one place. The downside is that the color-coding of log messages is lost.

[samreid]

It also looks like initialize-globals now depends on QueryStringMachine?

QSM is now included in every simulation, but I don't see any references to it in initialize-globals.js, am I missing something?

[pixelzoom]

@samreid You may want to review phetsims/beers-law-lab@087f9e1, since Beers Law Lab has some query parameters that are used by PhET-iO. I did test all query parameters and they appeared to behave as expected.

[pixelzoom]

The following common-code repositories use getQueryParameter, and I've indicated the number of uses. We should decide which ones to convert to QueryStringMachine, and put developer names next to them.

brand 1 chipper 17 joist 34 phet-io 27 phetcommon 2 scenery-phet 1 sun 2 tandem 1

[EDIT: Query parameters in all of the above will be consolidated in chipper.QueryParameters. See https://github.com/phetsims/chipper/issues/516.)

[pixelzoom]

While converting my sims, I also noticed:

(4) For type: 'number' there are some common validations (range, integer,...) that the validValues option doesn't support well or at all. So the client must do validation outside of QueryStringMachine, something like this:

var MyQueryParameters = QueryStringMachine.getAll( {
  numberOfWidgets: { type: 'number' }
} );

if ( !(MyQueryParameters.numberOfWidgets > 0 && MyQueryParameters.numberOfWidgets <= 100) )  {
  throw new Error( 'numberOfWidgets is out of range: ' + MyQueryParameters.numberOfWidgets );
}

return MyQueryParameters;

I think it would be useful to add something to QueryStringMachine to support this.

[jonathanolson]

QSM is now included in every simulation, but I don't see any references to it in initialize-globals.js, am I missing something?

It includes getQueryParameter calls for brand and locale, which would be converted to QSM, correct?

[samreid]

So the client end up needing to do validation outside of QueryStringMachine

Can you use isValidValue like this:

https://github.com/phetsims/query-string-machine/blob/438cfac1d9a37981ae6d9166a657b44915e1f018/js/test/testQueryStringMachine.js#L19-L25

[pixelzoom]

Ah, thanks, missed that.

[samreid]

It includes getQueryParameter calls for brand and locale, which would be converted to QSM, correct?

Yes, initialize-globals should be rewritten to use QueryStringMachine.

[pixelzoom]

There's a problem with isValidValue, tracking in #16.

[jessegreenberg]

@andrewadare suggested that we add a lint rule to find deprecated uses of getQueryParameter.

[jessegreenberg]

@pixelzoom volunteered to tackle joist.

[pixelzoom]

Other 10/27/16 notes: • Add @deprecated annotation to getQueryParameter • No new code should use getQueryParameter • Convert to QueryStringMachine as we go

[pixelzoom]

Query parameters for all "post-load" common code will be consolidated in chipper.QueryParameters. See phetsims/chipper#516.

[pixelzoom]

I created a separate issue for converting each sim to QueryStringMachine, to be addressed by the primary sim developer.

[pixelzoom]

I converted all sims that had a *QueryParameters.js file, but didn't do the ones that don't follow PhET convention for query parameters.

[pixelzoom]

Closing this issue. There are separate issues for each sim (see above), and for common code (https://github.com/phetsims/chipper/issues/516).