Easy post-load checking of namespacing

[jonathanolson]

As an alternative to https://github.com/phetsims/tasks/issues/435 (require.js-based) or https://github.com/phetsims/tasks/issues/431 (eslint-based) for checking the namespacing of https://github.com/phetsims/tasks/issues/378.

We had already created a debugging tool for creating these, and I've repurposed it so that if you run in require.js mode with ?ea&checkNamespaces, it will check require.js modules AFTER they all have been loaded, verifying that they exist at their expected namespace locations.

[jonathanolson]

Added to discuss using this as our sole namespace checking method (and enabling by default once we have all common code namespaced). This would mean bailing on https://github.com/phetsims/tasks/issues/435 (require.js-based namespace checks) and https://github.com/phetsims/tasks/issues/431 (eslint-based namespace checks).

[pixelzoom]

How do I test drive this? I tried:

http://localhost/~cmalley/Github/hookes-law/hookes-law_en.html?ea&checkNamespaces

... and I get this assertion failure:

Is this because joist isn't namespaced yet?

[samreid]

Yes, it asserts that all of the requirejs modules appear in the global namespace with the appropriate name and value. It is failing on the module "checkNamespaces" which has not been namespaced.

[samreid]

You can negate the assert conditions and transform them to if() console.log() if you want to see a full list of offenders.

[samreid]

I asked @jonathanolson over skype:

Brainstorming a change (improvement?) for checkNamespaces.js. How about it first prints out all offending namespace issues in one pass before assert()ing anything?

[samreid]

@jonathanolson responded:

That change sounds fine to me

[samreid]

How about it first prints out all offending namespace issues in one pass before assert()ing anything?

I have done this on my working copy but want to test it further before committing. Tomorrow.

[samreid]

Fixed above. Leaving open and marked for developer-meeting for discussion of how to use this and its limitations (only checks AMD modules loaded without a plugin).

[pixelzoom]

Very odd that checkNamespace by itself does nothing. Why does this features require 2 query parameters? (?ea&checkNamespaces)

How about checkNamespace to get the console output without assertion failure, and add ea to enable related assertions?

[samreid]

Consensus: We will keep the checkNamespace option for now to help developers namespace sims, in the future we will remove ?checkNamespaces and just use ?ea to throw assertion errors if a namespacing issue occurs.

[pixelzoom]

Limitations: • won't confirm namespacing of inner types • there are some non-require things that need to be namespaced in scenery

We can live with these.

[pixelzoom]

In Skype, I proposed that it's time to enable namespace checking. Discussion:

[5/2/16, 12:54:37 PM] Chris Malley: Re turning on checkNamespace... I see lots of recent code (expression-exchange-basics, CCK, making-tens,...) that is not namespaced. Looks like developers have been generally blowing this off. Is that what we decided to do? [5/2/16, 12:56:03 PM] Chris Malley: I understand for old code, but I'm a bit disappointed for recent code. [5/2/16, 12:56:48 PM] Sam Reid: For new code I often work in phases where I tackle cross cutting concerns such as i18n and namespacing as separate step. CCK isn’t i18n’ed yet either. Flipping the switch to require namespaces will encourage me to get to that step much sooner! (and I’m OK with that) [5/2/16, 12:59:23 PM] Chris Malley: How about new sims, like Neuron? 1.0.0 published in Feb, a full 3 months after we decided to namespace. There's 1 source file that's properly namespaced. [5/2/16, 12:59:39 PM] John Blanco: Me too - it'll force me to get these things done (or delegate some of them to Aadish).

[pixelzoom]

Assigned to @jonathanolson to review. Then I believe this can be closed.

[pixelzoom]

@phet-steele FYI, here's the current state of affairs when running test-server. The majority of runnable (sims and demos) are failing with "Assertion failed: not namespaced".

[pixelzoom]

I namespaced blast, chains, griddle, graphing-quadratics, ohm-law, resistance-in-a-wire.

[jonathanolson]

I'll plan to tackle build-a-molecule, making-tens, pendulum-lab, wave-on-a-string.

[jbphet]

Neuron, Area Builder, and Arithmetic have been namespaced. The 3 sims that comprise the States of Matter suite have been assigned to @aadish, see https://github.com/phetsims/states-of-matter/issues/114. I'll tackle Balancing Act and Expression Exchange tomorrow.

[jessegreenberg]

I namespaced balloons-and-static-electricity, friction, estimation, and faradays-law.

[samreid]

I namespaced cck in the above commit.

[samreid]

I namespaced bending-light in the above commit.

[samreid]

I namespaced protein-synthesis in the above commit.

[samreid]

I namespaced beaker in the above commit.

[samreid]

I namespaced energy-skate-park-basics in the above commit.

[samreid]

I namespaced fluid-pressure-and-flow in the above commit.

[samreid]

I namespaced seasons in the above commit.

[samreid]

Here's the new list of what's left (17 to go):

[jessegreenberg]

I am going to work on gravity-and-orbits and john-travoltage next.

[aadish]

namespaced build an atom in above commit

[jbphet]

Balancing Act and Expression Exchange are now namespaced.

[aadish]

States of Matter, Atomic Interactions and States of Matter Basics are namespaced

[jessegreenberg]

john-travoltage and gravity-and-orbits have been namespaced.

[phet-steele]

Hey all, still left:

• [x] balancing-act (@jbphet)
• [ ] build-a-molecule @jonathanolson or anyone else that wants to jump in
• [x] build-an-atom (@aadish)
• [x] calculus-grapher (@pixelzoom)
• [x] curve-fitting (@pixelzoom)
• [x] fraction-comparison (@pixelzoom)
• [x] fraction-matcher (@pixelzoom)
• [x] making-tens @jonathanolson or anyone else that wants to jump in (done by @pixelzoom)
• [x] pendulum-lab @jonathanolson or anyone else that wants to jump in (done by @pixelzoom)
• [x] plinko-probability @andrewadare (done by @pixelzoom)
• [x] projectile-motion (@jbphet)

[aadish]

my bad pushed the commit for build and atom on wrong branch

[jbphet]

Should now be fixed for Balancing Act. I had done the same thing as @aadish, i.e. I committed the changes on a branch instead of on master.

[jbphet]

Marking for developer meeting so that we can discuss and assign individuals to finish the remaining sims.

[jbphet]

I went ahead and did projectile-motion, since I set it up initially and there isn't much to it yet.

[pixelzoom]

I took care of all remaining sims, with one exception. build-a-molecule has its own non-standard namespace, and will need to be converted by @jonathanolson (tracking in https://github.com/phetsims/build-a-molecule/issues/78).

[pixelzoom]

@phet-steele Please kick off an automated test. Verify that the only runnable that fails with "Assertion failed: not namespaced" is build-a-molecule. If verified, close this issue. Otherwise note runnable that still need attention. Thanks.

[phet-steele]

build-a-molecule is all that's left! Congrats! :tada: :cake: :+1: :smile:

[pixelzoom]

Excellent, thanks.