-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convert codebase to use ES6 import/export modules instead of requirejs #820
Comments
I think that a quick (15 min?), high-level discussion of this in dev meeting is fine. But there a lot of questions, and I don't think it makes sense to involve the entire development team in a deep dive. If this is a priority for PhET, then I propose that a sub-team be formed to investigate. |
12/19/19 dev meeting: Pros:
Cons:
Feasibility test 1 - what are the problems?
Feasibility test 2 - investigate production needs
Adoption:
The development team agrees that this must happen, and should be scheduled into the project management plan. The question is when. @samreid and @jonathanolson will each put in 10 hours (sprint) on "Feasibility test 1" by mid January. Then we'll revisit and discuss with @kathy-phet if proceeding looks feasible. |
I added a script that migrates repos to use import/export. It is very prototype-y, but here's how it can be used for example-sim.
The result of running those steps is that it ports example-sim and all its dependencies to use import/export and builds it. It uses image loader and json loader. i18n plugin is ignored and mipmaps just use image loader at the moment. It runs in the browser for me, and works nicely. The mipmap dimensions are wrong, so the layout is incorrect. Brief investigation of highlighting/navigation/parameter popup works great for ES6 classes, not so much for places where we use Do not run the following step without understanding that revert-migrate does a hard reset on selected repos. That being said, iteration can be done via:
@jonathanolson and I were surprised to see that a build (without uglify) takes only a few seconds, at least for example-sim: real 0m2.342s Earlier today @pixelzoom and I wondered if using ES6 module import/export would help us with automatic imports, and it appears that it does (again, only for es6 classes, not for inherit types), and it's great that it uses aliases. UPDATE: A reminder to myself that I had to tell Webstorm about the config file like so: https://stackoverflow.com/questions/34943631/path-aliases-for-imports-in-webstorm even though the top answer says it should be automatic. UPDATE: I also tested by switching soundEnabled to true in a few places, and disabling assertions (due to a sizing error in the navbar), and sound is playing when I press the reset all button. |
Notes from collaboration with @jonathanolson: Compilation-free Mode proposal:
Compilation-only Mode proposal:
|
While looking into fast or incremental recompilation, we found that webpack supports a concept called hot module replacement, which allows you to edit code without reloading the entire application. With if ( module.hot ) {
module.hot.accept( './BarMagnetNode.js', () =>{
self.removeChild( barMagnetNode );
barMagnetNode = new BarMagnetNode( model.barMagnet, modelViewTransform );
self.addChild( barMagnetNode );
} );
} I am able to edit the code with a live, running, stateful simulation, like so: Please note, I am editing from my IDE, not from the chrome devtools, and a rebuild is taking place before swapping the code in the browser. Note the large logo size is because we haven't got mipmaps working yet. |
Can you clarify what this means? Does that mean that everyone must use an identical URL for running sims out of their working copy? If so, that's going to be tricky for sharing my working copy with VMware. I currently need to use the IP address of the host machine, not localhost. So my URL looks like http://192.168.1.2/phet/, not http://localhost/phet/. |
You would be able to use different hostname and port, but across developers the path would have to be the same. For instance, I serve |
Good to hear that incremental build + live reload is fast. If we do need to go with "compilation-free" mode, please consider 2 things when making the decision on URL path. (1) On macOS, it's very easy (well-documented) and conventional to configure Apache to serve local websites out of ${HOME}/Sites/. (2) Some of us (me included) may have non-PhET websites that we need to serve locally. Therefore, the path to my PhET working directory on macOS is http://localhost/~cmalley/GitHub/. The path to the local copy of my company website is http://localhost/~cmalley/pixelzoom/. Etc. Trying to flatten things out to something like http://localhost/phetmarks/ would be a big pain for me, across multiple machines and VMs. |
Notes from discussion with @jonathanolson. Question: How can we support jumping around to different simulations?
a patch Index: perennial/bin/revert-migrate.sh
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- perennial/bin/revert-migrate.sh (revision 754be48a6d0fa438c07d51859a6baa478ba6bd57)
+++ perennial/bin/revert-migrate.sh (date 1577125467447)
@@ -1,56 +1,376 @@
-#!/bin/bash
+cd ../acid-base-solutions
+git reset --hard
+
+cd ../area-builder
+git reset --hard
+
+cd ../area-model-algebra
+git reset --hard
+
+cd ../area-model-decimals
+git reset --hard
+
+cd ../area-model-introduction
+git reset --hard
+
+cd ../area-model-multiplication
+git reset --hard
+
+cd ../arithmetic
+git reset --hard
+
+cd ../atomic-interactions
+git reset --hard
+
+cd ../balancing-act
+git reset --hard
+
+cd ../balancing-chemical-equations
+git reset --hard
+
+cd ../balloons-and-static-electricity
+git reset --hard
+
+cd ../beers-law-lab
+git reset --hard
+
+cd ../bending-light
+git reset --hard
+
+cd ../blackbody-spectrum
+git reset --hard
+
+cd ../blast
+git reset --hard
+
+cd ../build-a-fraction
+git reset --hard
+
+cd ../build-a-molecule
+git reset --hard
+
+cd ../build-an-atom
+git reset --hard
+
+cd ../bumper
+git reset --hard
+
+cd ../buoyancy
+git reset --hard
+
+cd ../calculus-grapher
+git reset --hard
+
+cd ../capacitor-lab-basics
+git reset --hard
+
+cd ../chains
+git reset --hard
+
+cd ../charges-and-fields
+git reset --hard
+
+cd ../circuit-construction-kit-ac
+git reset --hard
+
+cd ../circuit-construction-kit-black-box-study
+git reset --hard
+
+cd ../circuit-construction-kit-dc
+git reset --hard
+
+cd ../circuit-construction-kit-dc-virtual-lab
+git reset --hard
+
+cd ../color-vision
+git reset --hard
+
+cd ../collision-lab
+git reset --hard
+
+cd ../concentration
+git reset --hard
+
+cd ../coulombs-law
+git reset --hard
+
+cd ../curve-fitting
+git reset --hard
+
+cd ../density
+git reset --hard
+
+cd ../diffusion
+git reset --hard
+
+cd ../energy-forms-and-changes
+git reset --hard
+
+cd ../energy-skate-park
+git reset --hard
+
+cd ../energy-skate-park-basics
+git reset --hard
+
+cd ../equality-explorer
+git reset --hard
+
+cd ../equality-explorer-basics
+git reset --hard
+
+cd ../equality-explorer-two-variables
+git reset --hard
+
+cd ../estimation
+git reset --hard
+
+cd ../example-sim
+git reset --hard
+
+cd ../expression-exchange
+git reset --hard
+
+cd ../faradays-law
+git reset --hard
+
+cd ../fluid-pressure-and-flow
+git reset --hard
+
+cd ../forces-and-motion-basics
+git reset --hard
-#=======================================================================================
-#
-# Resets the repos associated with the migrate feasibility prototype, see https://github.com/phetsims/chipper/issues/820
-#
-# Author: Sam Reid
-#
-#=======================================================================================
+cd ../fraction-comparison
+git reset --hard
-cd ../axon
+cd ../fraction-matcher
+git reset --hard
+
+cd ../fractions-equality
+git reset --hard
+
+cd ../fractions-intro
+git reset --hard
+
+cd ../fractions-mixed-numbers
+git reset --hard
+
+cd ../friction
+git reset --hard
+
+cd ../function-builder
+git reset --hard
+
+cd ../function-builder-basics
+git reset --hard
+
+cd ../gas-properties
+git reset --hard
+
+cd ../gases-intro
+git reset --hard
+
+cd ../gene-expression-essentials
+git reset --hard
+
+cd ../graphing-lines
+git reset --hard
+
+cd ../graphing-quadratics
+git reset --hard
+
+cd ../graphing-slope-intercept
+git reset --hard
+
+cd ../gravity-and-orbits
+git reset --hard
+
+cd ../gravity-force-lab
+git reset --hard
+
+cd ../gravity-force-lab-basics
+git reset --hard
+
+cd ../griddle
git reset --hard
-cd ../brand
+cd ../hookes-law
git reset --hard
-cd ../dot
+cd ../interaction-dashboard
+git reset --hard
+
+cd ../isotopes-and-atomic-mass
+git reset --hard
+
+cd ../john-travoltage
git reset --hard
cd ../joist
git reset --hard
-cd ../kite
+cd ../least-squares-regression
git reset --hard
-cd ../phetcommon
+cd ../make-a-ten
git reset --hard
-cd ../phet-core
+cd ../masses-and-springs
git reset --hard
-cd ../phet-io
+cd ../masses-and-springs-basics
git reset --hard
-cd ../example-sim
+cd ../models-of-the-hydrogen-atom
git reset --hard
-cd ../scenery
+cd ../molarity
+git reset --hard
+
+cd ../molecules-and-light
+git reset --hard
+
+cd ../molecule-polarity
+git reset --hard
+
+cd ../molecule-shapes
+git reset --hard
+
+cd ../molecule-shapes-basics
+git reset --hard
+
+cd ../natural-selection
+git reset --hard
+
+cd ../neuron
+git reset --hard
+
+cd ../number-line-integers
+git reset --hard
+
+cd ../number-play
+git reset --hard
+
+cd ../ohms-law
+git reset --hard
+
+cd ../optics-lab
+git reset --hard
+
+cd ../pendulum-lab
+git reset --hard
+
+cd ../ph-scale
+git reset --hard
+
+cd ../ph-scale-basics
+git reset --hard
+
+cd ../phet-io-test-sim
+git reset --hard
+
+cd ../plinko-probability
+git reset --hard
+
+cd ../projectile-motion
+git reset --hard
+
+cd ../proportion-playground
+git reset --hard
+
+cd ../reactants-products-and-leftovers
+git reset --hard
+
+cd ../resistance-in-a-wire
+git reset --hard
+
+cd ../rutherford-scattering
git reset --hard
cd ../scenery-phet
git reset --hard
+
+cd ../simula-rasa
+git reset --hard
+
+cd ../states-of-matter
+git reset --hard
+
+cd ../states-of-matter-basics
+git reset --hard
cd ../sun
git reset --hard
cd ../tambo
git reset --hard
+
+cd ../tappi
+git reset --hard
+
+cd ../trig-tour
+git reset --hard
+
+cd ../twixt
+git reset --hard
+
+cd ../under-pressure
+git reset --hard
+
+cd ../unit-rates
+git reset --hard
+
+cd ../vector-addition
+git reset --hard
+
+cd ../vector-addition-equations
+git reset --hard
+
+cd ../vegas
+git reset --hard
+
+cd ../vibe
+git reset --hard
+
+cd ../wave-interference
+git reset --hard
+
+cd ../wave-on-a-string
+git reset --hard
+
+cd ../waves-intro
+git reset --hard
+
+cd ../wilder
+git reset --hard
+
+cd ../axon
+git reset --hard
+
+cd ../scenery
+git reset --hard
+
+cd ../brand
+git reset --hard
cd ../tandem
git reset --hard
+
+cd ../phet-core
+git reset --hard
+
+cd ../dot
+git reset --hard
+
+cd ../kite
+git reset --hard
cd ../utterance-queue
git reset --hard
-cd ../example-sim
\ No newline at end of file
+cd ../phetcommon
+git reset --hard
+
+cd ../phet-io
+git reset --hard
+
+cd ../example-sim
Index: chipper/js/grunt/migrate.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- chipper/js/grunt/migrate.js (revision 6479b33911a10f0b1651e7a405bb5a74f3129f6d)
+++ chipper/js/grunt/migrate.js (date 1577128388899)
@@ -18,6 +18,39 @@
return str.split( search ).join( replacement );
};
+/**
+ *
+ * const lightBulbImage = require( 'image!ACID_BASE_SOLUTIONS/light-bulb-icon.png' );
+ * import lightBulbImage from 'ACID_BASE_SOLUTIONS/../images/light-bulb-icon.png';
+ *
+ * @param {string} contents
+ */
+const replaceImageStatements = contents => {
+ const prefix = 'require( \'image!';
+ let index = 0;
+ while ( contents.indexOf( prefix, index ) >= 0 ) {
+ const innerIndex = contents.indexOf( prefix );
+ let nextIndex = contents.indexOf( '/', innerIndex + 1 );
+ const start = contents.substring( 0, nextIndex );
+ contents = start + '/../images/' + contents.substring( nextIndex + 1 );
+ index += prefix.length + 1;
+ }
+ return contents;
+};
+
+const replaceMipmapStatements = contents => {
+ const prefix = 'require( \'mipmap!';
+ let index = 0;
+ while ( contents.indexOf( prefix, index ) >= 0 ) {
+ const innerIndex = contents.indexOf( prefix );
+ let nextIndex = contents.indexOf( '/', innerIndex + 1 );
+ const start = contents.substring( 0, nextIndex );
+ contents = start + '/../images/' + contents.substring( nextIndex + 1 );
+ index += prefix.length + 1;
+ }
+ return contents;
+};
+
const migrateFile = async ( repo, relativeFile ) => {
if ( relativeFile.endsWith( '/PhetioIDUtils.js' ) ) {
return;
@@ -32,7 +65,11 @@
contents = replace( contents, '= require( \'ifphetio!', '= function(){return function(){ return function(){}; };}; // ' );
contents = replace( contents, 'require( \'mipmap!BRAND/logo.png\' )', 'require( \'BRAND/../images/logo.png\' ).default' );
contents = replace( contents, 'require( \'mipmap!BRAND/logo-on-white.png\' )', 'require( \'BRAND/../images/logo-on-white.png\' ).default' );
- contents = replace( contents, 'require( \'image!EXAMPLE_SIM/barMagnet.png\' )', 'require( \'EXAMPLE_SIM/../images/barMagnet.png\' ).default' );
+
+ contents = replaceImageStatements( contents );
+ contents = replaceMipmapStatements( contents );
+
+ // contents = replace( contents, 'require( \'image!EXAMPLE_SIM/barMagnet.png\' )', 'require( \'EXAMPLE_SIM/../images/barMagnet.png\' ).default' );
contents = replace( contents, 'require( \'mipmap!JOIST/keyboard-icon-on-white.png\' )', 'require( \'JOIST/../images/keyboard-icon-on-white.png\' ).default' );
contents = replace( contents, 'require( \'mipmap!JOIST/keyboard-icon.png\' )', 'require( \'JOIST/../images/keyboard-icon.png\' ).default' );
contents = replace( contents, 'require( \'sound!TAMBO/empty_apartment_bedroom_06_resampled.mp3\' )', 'require( \'TAMBO/../sounds/empty_apartment_bedroom_06_resampled.mp3\' ).default' );
@@ -100,6 +137,10 @@
contents = replace( contents, `return inherit;`, `export default inherit;` );
contents = replace( contents, `' ).default;`, `';` );
+ contents = replace( contents, ` from 'image!`, ` from '` );
+ contents = replace( contents, ` from 'mipmap!`, ` from '` );
+
+ // import '/axon/js/myFile'
// contents = replace(contents,`from 'AXON/`,`from '/axon/js/`);
// contents = replace(contents,`from 'BRAND/`,`from '/brand/phet/js/`);
@@ -159,20 +200,130 @@
// const repos = fs.readFileSync( '../perennial/data/migrate-repos', 'utf-8' ).trim().split( /\r?\n/ ).map( sim => sim.trim() );
const repos = `axon
+scenery
brand
+tandem
+phet-core
dot
-joist
kite
+utterance-queue
phetcommon
-phet-core
phet-io
+acid-base-solutions
+area-builder
+area-model-algebra
+area-model-decimals
+area-model-introduction
+area-model-multiplication
+arithmetic
+atomic-interactions
+balancing-act
+balancing-chemical-equations
+balloons-and-static-electricity
+beers-law-lab
+bending-light
+blackbody-spectrum
+blast
+build-a-fraction
+build-a-molecule
+build-an-atom
+bumper
+buoyancy
+calculus-grapher
+capacitor-lab-basics
+chains
+charges-and-fields
+circuit-construction-kit-ac
+circuit-construction-kit-black-box-study
+circuit-construction-kit-dc
+circuit-construction-kit-dc-virtual-lab
+color-vision
+collision-lab
+concentration
+coulombs-law
+curve-fitting
+density
+diffusion
+energy-forms-and-changes
+energy-skate-park
+energy-skate-park-basics
+equality-explorer
+equality-explorer-basics
+equality-explorer-two-variables
+estimation
example-sim
-scenery
+expression-exchange
+faradays-law
+fluid-pressure-and-flow
+forces-and-motion-basics
+fraction-comparison
+fraction-matcher
+fractions-equality
+fractions-intro
+fractions-mixed-numbers
+friction
+function-builder
+function-builder-basics
+gas-properties
+gases-intro
+gene-expression-essentials
+graphing-lines
+graphing-quadratics
+graphing-slope-intercept
+gravity-and-orbits
+gravity-force-lab
+gravity-force-lab-basics
+griddle
+hookes-law
+interaction-dashboard
+isotopes-and-atomic-mass
+john-travoltage
+joist
+least-squares-regression
+make-a-ten
+masses-and-springs
+masses-and-springs-basics
+models-of-the-hydrogen-atom
+molarity
+molecules-and-light
+molecule-polarity
+molecule-shapes
+molecule-shapes-basics
+natural-selection
+neuron
+number-line-integers
+number-play
+ohms-law
+optics-lab
+pendulum-lab
+ph-scale
+ph-scale-basics
+phet-io-test-sim
+plinko-probability
+projectile-motion
+proportion-playground
+reactants-products-and-leftovers
+resistance-in-a-wire
+rutherford-scattering
scenery-phet
+simula-rasa
+states-of-matter
+states-of-matter-basics
sun
tambo
-tandem
-utterance-queue`.trim().split( /\r?\n/ ).map( sim => sim.trim() );
+tappi
+trig-tour
+twixt
+under-pressure
+unit-rates
+vector-addition
+vector-addition-equations
+vegas
+vibe
+wave-interference
+wave-on-a-string
+waves-intro
+wilder`.trim().split( /\r?\n/ ).map( sim => sim.trim() );
repos.forEach( ( repo, index ) => {
console.log( index + '/' + repos.length );
Index: chipper/js/all.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- chipper/js/all.js (date 1577125892624)
+++ chipper/js/all.js (date 1577125892624)
@@ -0,0 +1,2 @@
+import exampleSimMain from 'EXAMPLE_SIM/example-sim-main';
+import hello from '../../acid-base-solutions/js/acid-base-solutions-main';
\ No newline at end of file
|
After the commits, acid-base-solutions is nearly linting. |
I created a planning document here: https://docs.google.com/document/d/1_b8_DBP2Ier4fHkU_osOIjnQ6wdUtyjiifSznEzV4JM/edit |
Named exports/importsFor files like DOT/Utils or constants files, I think switching to also support named exports would be ideal. It's possible to have both default and named exports (since the default export literally exports something with the name // Utils.js
export function clamp( value, min, max ) {
// stuff
}
const Utils = {
clamp: clamp, // place it in the Utils object (extra verbose since we disallow the shortcut still)
// etc.
};
export default Utils; This allows a few different import styles, for example the default still works: import Utils from '../dot/js/Utils.js';
Utils.clamp( ... ); but also allows direct usage: import { clamp } from '../dot/js/Utils.js';
clamp( ... ); Notably either can be renamed ( Additionally, this would work well with constants files: // ABSConstants.js
const WEAK_STRENGTH_MAX = 1E2;
export const CONCENTRATION_RANGE = new RangeWithValue( 1E-3, 1, 1E-2 );
export const PH_RANGE = new Range( 0, 14 );
export const WATER_EQUILIBRIUM_CONSTANT = 1E-14;
export const WATER_CONCENTRATION = 55.6; // water concentration when it's used as a solvent, mol/L
export const WEAK_STRENGTH_RANGE = new RangeWithValue( 1E-10, WEAK_STRENGTH_MAX, 1E-7 );
export const STRONG_STRENGTH = WEAK_STRENGTH_MAX + 1; // arbitrary, but needs to be greater than weak max and we can keep the code for compatibility with the defaults, which could be removed when we don't need the default: const ABSConstants = {
CONCENTRATION_RANGE: CONCENTRATION_RANGE,
PH_RANGE: PH_RANGE,
WATER_EQUILIBRIUM_CONSTANT: WATER_EQUILIBRIUM_CONSTANT,
WATER_CONCENTRATION: WATER_CONCENTRATION,
WEAK_STRENGTH_RANGE: WEAK_STRENGTH_RANGE,
STRONG_STRENGTH: STRONG_STRENGTH
};
acidBaseSolutions.register( 'ABSConstants', ABSConstants );
export default ABSConstants; This allows: import { PH_RANGE } from '../../../../acid-base-solutions/js/common/ABSConstants.js';
PH_RANGE.min Additionally, if we prefer to NOT get rid of the import * as ABSConstants from '../../../../acid-base-solutions/js/common/ABSConstants.js';
ABSConstants.PH_RANGE.min So it's my feeling that this is better long-term than our current approach. I'm unclear on whether the "getting old things to that style" is worth the effort, but I'd like to use those new features for new simulations or code. Cyclic dependenciesFor those cases where we need to ensure a module loads (but we don't need to assign it to a value, potentially because we use the namespace system as a workaround), we should use an empty import, e.g. in Node.js, we need to ensure Trail.js is loaded: import '../../../scenery/js/util/Trail.js'; Additionally, cyclic dependencies should work well with our system in general (at least with my testing on the cases where I've run into things), so we should be safe to do this (loading in browser and built): // Vector2.js
import Vector3 from './Vector3.js';
// In Vector2's definition:
toVector3: function() {
return new Vector3( this.x, this.y, 0 );
},
// Vector3.js
import Vector2 from './Vector2.js';
// In Vector3's definition:
toVector2: function() {
return new Vector2( this.x, this.y );
}, So empty requires should turn into empty imports, and we should have steps after to move to cyclic imports where this helps for clarity (so we can avoid using our namespace system as a workaround). |
All work tracked in other issues, closing. |
In #551 we started moving simulation code to ES6, and that has been working nicely. One remaining step that we stand to benefit from is converting from requirejs to ES6 module import/export. When we began HTML5 development around 2013, ES6 was not finalized and there was no support for ES6 module import/export. Now it is the standard. It is supported on many platforms directly, and we can compile to ES5 compatibility for IE11:
When we converted from Java to HTML by way of AMD/requirejs, we lost most of our tooling support. For instance, in this code, WebStorm is unable to identify the parameters for the
Emitter
type, and code navigation only takes you to line 13, not to the definition of Emitter.Similarly, in this case, WebStorm cannot find
modelViewTransform.modelToViewX
, orcontainer.leftWallDoesWork
, and marks them as unresolvedThis is in part because WebStorm does not know how to parse AMD modules which we are using for requirejs. ES6 module import/export has much better support--it can show parameters:
and it does a much better job highlighting and navigating:
I want to stress that this is not a minor concern--better support for highlighting, navigation, autocomplete would be a significant help in day-to-day development. I don't know what benefits might be attained in Sublime or other environments.
Furthermore, moving to the established standard pattern will be beneficial for the long term, for communicating with other developers, bringing new team members up-to-speed, interoperability with other software, future-proofing, etc.
Some questions as we proceed:
I know we are deeply entrenched in our existing pattern and got it to work for us pretty well, but my hope is that with some effort we can build a stronger foundation to help streamline future development.
Let's discuss at an upcoming developer meeting.
The text was updated successfully, but these errors were encountered: