P5.js: ES6 Transition: Concerns & Progress

[hsab]

Hello everyone!

I'm investigating the means of fully transitioning p5 to ES6. I've already spoken to some of the developers outside of Github and was hoping to shine some light on some of the intricacies that I've observed in the code.

I've noticed there are 42 instances of module.exports = p5; in the bundled .js files. And I hope this issue serves as a clarification of the assumption that I have:

From what I understand each module.exports = p5; is effectively exposing p5 after extending it in its own respected files/module, be it curves.js, vertex.js, etc.

Although what seems to be the case is that most modules usually inherit/require from main.js, there are few instances of sequential inheritance of p5.

For instance:
In string_functions.js we have:

var p5 = require('../core/main');
require('../core/error_helpers');
....
module.exports = p5;

Both main and error_helpers have their own respective modules.exports = p5. However string_functions is exporting the instance of p5 inheritted from main.

My question in regards to this specific instance is by the end of string_functions when we are exporting p5, will this instance have all the prototypal functions defined in error_helpers?

From my understanding, the inheritance scheme seems to be a directed acyclical graph with a single root node (main.js - A in picture) and a single leaf node which is p5 at its final stage after all the additions to the prototype occur in multiple files ( F in picture or equivalently p5() in browser).

I know javascript is independent of the order the code is written unlike C++, and instances and references are resolved during JIT. Given that there are 42 instances of module.exports=p5 in the bundled lib, how is each instance resolved properly? Is each instance unique, with functionalities unavailable in preceding exports or do they all refer to the same class for lack of a better word?

Moreover given the import syntax in ES6, how does this export/require paradigm change, if it does?

Apologies in advance if these questions seem trivial or show a lack of knowledge in JS. I come from different programming languages and JS seems like wild west to me!

Edit: I'm unable to appropriately label my issue. I assume it should be considered a "discussion".

[Spongman]

There is only one p5 constructor function (‘class’), and it is passed around (and returned by) all the other various modules. There’s no inheritance going on (except for the Graphics stuff - ignore that), all the p5 methods are just added into the same prototype object (p5.prototype). It’s the JavaScript equivalent of C# partial classes, or #including parts of a single C++ class defined in separate files.

This is impossible to do in ES6-style class definitions.

[hsab]

My choice of word for "inheritance" was probably the worst imaginable. Mainly what I was referring to was this notion of partial classes and prototypal extensions.

I know that you can have ES6 classes split over multiple files through the same procedure as ES5 with Class.prototype.newFunction

What I'm wondering is if this can be applied to an imported module? And from what I'm gathering you seem to suggest that this is not doable right?

Would really appreciate if I can get a clarification on this:

Given that there are 42 instances of module.exports=p5 in the bundled lib, how is each instance resolved properly? Is each instance unique, with functionalities unavailable in preceding exports or do they all refer to the same class for lack of a better word?

[hsab]

This is impossible to do in ES6-style class definitions.

I managed to create a very simple working example of what I was alluding to as "ES6 prototypal extensions":

Directory structure:

.
├── public
│   └── index.html
├── src
│   └── app.js
│   └── foo.js

foo.js:

export class Foo {
    constructor() {
        console.log("Foo Constructed");
    }
}

app.js:

import {
    Foo
} from './foo';

Foo.prototype.instFunc = function () {
    console.log("Instance proto extension");
}

Foo.staticFunc = function () {
    console.log("Static proto extension");
}

Foo.staticFunc();
const foo = new Foo();
foo.instFunc();

Cmds:

$> babel src -d public
$> browserify ./public/app.js -o ./public/bundle.js 

index.html:

<script src="./bundle.js"></script>

This is being babelified with babel-preset-env and then bundled with browserify.
@Spongman do you foresee any issues with this paradigm in the long run? Particularly given that it mirrors how p5 is implemented atm. I've refrained from inheritance since ES6 classes seem to have a performance hit.
@outofambit will this structure introduce any difficulties with the babelify build system? I've tried to think of implementations that minimize code restructuring and this is pretty much the simplest way of replacing require with import, without dealing with sub/super classes and inheritance.

Next step is testing this with p5 and a subsample of the available modules.
Thanks in advance

[limzykenneth]

Given that there are 42 instances of module.exports=p5 in the bundled lib, how is each instance resolved properly? Is each instance unique, with functionalities unavailable in preceding exports or do they all refer to the same class for lack of a better word?

From what I understand, even though there are 42 occurance of module.exports=p5 in the final bundled lib, they all refer to the same instance of p5. As @Spongman mentioned the one single instance is passed around and returned by each module and exposed globally as p5.

My question in regards to this specific instance is by the end of string_functions when we are exporting p5, will this instance have all the prototypal functions defined in error_helpers?

I have limited understanding of how a javascript engine works at the very lowest level so I may be wrong, but to answer your question, in a way yes. When talking about prototypal properties and whether they are available or not we will be looking at the instanced p5 class ie new p5(), and that will be from app.js from the source root. You can see from there that it will have all the required partial class definition. The order doesn't matter as the javascript engine will parse the entire thing and all valid references will be available regardless of the order they are defined in. To be more specific, in the file that defined string_functions you will have access to prototypal methods defined in error_helpers as they will both be refering to the same p5 instance that has been parsed by the engine.

The syntax you proposed for ES6 prototypal extension will probably work (I'm not sure, I've use ES5 much longer than I've use ES6) but in that case only one file will be using ES6 class syntax (foo.js) every thing else will be using prototype ie ES5. Personally I don't have much of a problem with this but it kind of defeat the purpose of using ES6 classes in the first place as it doesn't clean up much of the source.

[hsab]

Thank you for your wonderful and informative response.

I think the codebase could benefit from many of ES6 facilities, such as arrow functions, iterators, parameter handlers, etc. Which not only make development easier in the long run but lower the barrier of entry for contributors.

As for the class syntax, I wholeheartedly agree with you, but I think the reason for my interest in sticking to the prototypal injection has to do with first performance and second a much more fundamental design pattern that I see throughout the codebase.

Performance: I've come across many articles and experiments that claim ES6 classes, particularly when instanced in large numbers have a heavy overhead and consequent performance hit. Six Speed shows some interesting and alarming figures. I'm sure browsers and JS engines will catch up soon, but as of writing this, I have to admit that I hardly see ES6 code deployed or used in production, as it's always transpiled to ES5 and I haven't been able to obtain any information regarding the performance of ES6 classes once they are transpiled to ES5 prototypes. So my initial objection to classes has to do with these reports of performance.

Code Design: Please take the following with a grain of salt as I don't know the codebase in its entirety and correct me if I'm wrong about this, but from what I do know of the code and what you and others have clarified here it seems that a full-fledged transition to ES6 classes requires a rethinking of p5 modules and the current architecture. What seems to be the case is that p5 is a massive class/object with many functions/attributes injected to the prototype through different modules. Moreover, there seems to be no real _class_ hierarchy associated with these modules, other than the fact that they are broken into different files based on semantic and functional distinctions.

To properly transition to classes, p5 needs to be class-based or at least prototype-based. And although at first it might seem like p5 falls into the latter form, upon further inspection one can see that virtually no inheritance is going on. I'm not sure what to call p5's paradigm, it borrows a bit from functional programming, prototypal programming and scripting, but I certainly don't think it adheres to OOP principles that permit a smooth transition to ES6's class syntax. I do recognize that design choices were based on the limitations of JS at the time, however given the relevance of ES6 and where JS is headed, an appropriate restructuring and design architecture needs to be proposed that effectively transitions p5 to OOP in order to properly utilize classes and reap their benefits. Until then I believe using some of the other facilities of ES6 is a step in the right direction.

[limzykenneth]

Performance
ES6 class is mostly syntactic sugar on top of the existing prototypal object orientation JS have, if you look at the performance benchmark chart you provided, ES6 classes actually doesn't have any performance difference when running purely as ES6 classes, only transpiled classes have some performance hit (probably because of polyfills). We can opt for not transpiling class syntax to prototype but actually the constructor is not something that is called all that much often to be a performance bottle neck. Most of the time the reason ES6 code is not deployed in production is not out of concern for performance but rather browser support, many websites still wants to provide support for old browsers (read IE).

Code Design
In the end, the choice to use class or prototype is down to design choices. p5 is unique in away (and different from many other libraries) in that many other libraries breakout their functionalities into their own object definition which are then individually attached to the exposed global object (three.js API comes to mind), while in p5 most functionalities are attached to the same global object which lets it expose a more abstracted API to the end user, ie you can do ellipse(50, 50, 100, 100) instead of new Ellipse(50, 50, 100, 100), but still keep the codebase relatively separated.

To quote an often quoted line from MDN

JavaScript classes, introduced in ECMAScript 2015, are primarily syntactical sugar over JavaScript's existing prototype-based inheritance. The class syntax does not introduce a new object-oriented inheritance model to JavaScript.

I think it is important to remember that ES6 classes are still prototypes and prototypes very likely isn't going anywhere regardless of what future development Javascript may have. Anything that can be done with ES6 class can be done with prototypes (with varying degrees of complexity), but not necessarily the reverse.

I don't think p5 should strive to be too OOP since to adhere to OOP pattern would mean exposing new Ellipse(50, 50, 100, 100) as the syntax for most operations. It is much more important to expose a low barrier of entry API to the end user than to try to use a particular feature such as class syntax.

However, as you've said many other features of ES6 can definitely be added into the codebase.

[hsab]

Thanks again for an informative and prompt response.

I'm not sure how I interpreted the performance tests. But you're absolutely right. Pure ES6 has identical performance for classes. And reading myself again I was making no sense.

We can opt for not transpiling class syntax to prototype

Wouldn't this cause issues with older browsers e.g. IE11? How are older browsers dealt with atm? I was under the impression that p5 supported down to IE8.

I'm aware of classes being eye-candy wrappers for prototype inheritance. Although it might have sounded like advocating for objectifying every facility of p5, in fact in a conversation with @lmccart I was very impressed of how p5 deals with many functions, particularly what you mention, like ellipse and rectangle among many more in that they behave in a semi static way rather than instancing an object for every call. I can't imagine spawning 1000 new Ellipses in the draw function and how it effects the garbage collector.

What I was mainly confused about is how do we deal with the inheritance hierarchy. Take Image as an example. What sub classes does it inherit from, if any? and how do we go about defining those? Are filters their own class, do they inherit from Image or are filters implemented in the Image class itself? Moreover what classes inherit from Image? Would texture fall into that category? Or maybe there's a base class that both Image and Texture inherit from?

Overall my concern was associated with the inheritance architecture, hierarchy and how the resulting directed graph ends up looking. Such a hierarchy is quite ambiguous atm although the organization of src folder seems to hint at how it is imagined.

However, as you've said many other features of ES6 can definitely be added into the codebase.
I have a branch where I'm making some ES6 transitions. I hope to have a babelified+browserified version by Saturday to test and benchmak. Will report back here.

[limzykenneth]

Wouldn't this cause issues with older browsers e.g. IE11? How are older browsers dealt with atm? I was under the impression that p5 supported down to IE8.

After the recent merge of babelify into the build, the support is now according to browserslist last 2 versions and not dead, as such for IE, only IE11 is supported and no lower.

What I was mainly confused about is how do we deal with the inheritance hierarchy. Take Image as an example. What sub classes does it inherit from, if any? and how do we go about defining those? Are filters their own class, do they inherit from Image or are filters implemented in the Image class itself? Moreover what classes inherit from Image? Would texture fall into that category? Or maybe there's a base class that both Image and Texture inherit from?

Most of the p5 code base doesn't do much inheritence (as everything is directly attached to the same p5 prototype), the main ones that do are those that relates to canvases/renderers/images.

[outofambit]

Thanks for this great discussion @hsab and @limzykenneth! I want to be careful that we don't try to take on / decide too much at once. While import/export and class syntax both (can) impact the structure of the library, they are not dependent on another. @hsab what do you think about having a separate thread to track discussions around using class syntax?

---

I did some experiments with import/export in the code base today (using VSCode's auto-refactor feature) and encountered some issues with the build system. 🎉😭

Quick summary of issues i ran into:

• eslint needed a config tweak to parse es6 syntax properly
• discovered browserify transforms were in the wrong order to fully support es6 (that's my bad)
• brfs doesn't like import syntax for inlining shader assets
• current build is not exporting p5 object properly for script tags (still investigating)

You can see my changes here: https://github.com/processing/p5.js/compare/master...outofambit:require-to-import

I'll keep pushing on making sure the build system can support this and update with progress!

[Spongman]

@outofambit this is good stuff, although i have to say i'm not a big fan of importing all the symbols directly into the ambient namespace. i'd much rather retain the current use of writing constants.TWO_PI. it's purely aesthetic, but it just seems to me like it's not making the best use of the module system to be merging symbols into one big bucket.

[hsab]

@outofambit thanks for these changes. class definitely needs its own discussion thread. I by no means intended to make any modifications on that level. Like we discussed earlier my focus is primarily on syntactical migration.

On another note, I agree with @Spongman on constants. I assume the current result is due to the auto-refractor feature.

While testing your require-to-import branch, I had some issues with the lib not working properly on the browser with the empty-example. They were related to hasOwnProperty and I managed to fix with this commit.

While investigating the error p5 is not a constructor being thrown for every single unit test, I noticed the library is working globally but not in instanced mode. I'm still investigating this, but so far what I've found:

• the constructor is only accessible through p5.default(...)

• reverting export default p5; to module.exports = p5; in app.js fixes the issue.

  • I stumbled on this and diff-ed for each syntax:

  $ diff p5-module-exports.js p5-export-default.js
    49527,49528d49526
    <           Object.defineProperty(exports, '__esModule', { value: true });
    <           exports.default = void 0;
    49614,49615c49612
    <           var _default = _main.default;
    <           exports.default = _default;
    ---
    >           module.exports = _main.default;
  
  

  I suspected maybe this has something to do with the standalone option of browserify. Seems like it doesn't, but while looking around found this issue on babelify and seems like this is the expected behavior.

Will investigate more on how to bring the p5() constructor back while using export default p5; or similar ES6 syntax.

Edit: Was wondering if brfs-babel could address the import issues with brfs

This provides some new features, like ES2015 import support...

[outofambit]

@hsab i've incorporated your commit and the fix for the constructor is only accessible through p5.default(...). thank you! i did a little more cleanup, too.

i think leaving module.exports = p5; in app.js is totally fine. (i think it would be more effort and more complicated to find some other build solution to work around this.)

On another note, I agree with @Spongman on constants. I assume the current result is due to the auto-refractor feature.

yeah i totally agree that's not ideal and is indeed from the auto-refactor.

@hsab would you like to take it from here with the https://github.com/processing/p5.js/compare/master...outofambit:require-to-import branch and open a pull request? i think we are now in the clear on build issues for this. (i didn't fix the constants importing thing and i think there's more adjusting that could be done with src/core/helpers.js and src/core/constants.js and similarly structured files.)

[hsab]

@outofambit Sorry for the late response. I've been super busy with school.
I have incorporated all of your adjustments in https://github.com/processing/p5.js/compare/master...es6
Unfortunately, I decided to start from the latest master to be semi-uptodate, and because of this I manually replicated your modifications.

Here's a list of what has been done so far:

• [x] Every instance of _require_ has been changed to _import_. (both node, e.g. fs, and p5 modules).
• [x] Constants are imported such that syntax replicates old format: constants.TWO_PI
• [x] Every module is exported in ES6 style.
  Including src/core/helpers.js and src/core/constants.js
  With the exception of app.js still using module.exports = p5;
• [x] Using @babel/register to properly perform mochaTest:test suite with babel as a compiler.
• [x] Replaced brfs with babel-plugin-static-fs
brfs is completely removed as a dependency.
  static-fs allows for import { readFileSync } from 'fs';
• [x] Fixed combineModules.js to properly handle the migration to import syntax.

I've tested this in both global and instanced modes and everything seems to be working as expected without any modification to existing examples/test.

Next, I will perform more syntactic migrations aligned with ES6.

[outofambit]

@hsab that’s awesome! sounds like a lot of great progress! thanks for the update!

what are you thinking in terms of merging this work in? i’m a little nervous about having one giant pull request for this, especially due to merge conflicts with ongoing work. do you think its possible to do this in multiple pull requests?

[Spongman]

Please don’t add usage of lambda expressions/forEach to the library code. There is a significant performance penalty when using them.

[lmccart]

@Spongman can you point us to any references that document this performance difference? jsperf tests or similar? @hsab is looking into this as well.

[Spongman]

yeah, a simple in-browser test will show that using functional style is at least 3 times as expensive as a for loop: https://codepen.io/Spongman/pen/XLYPjL?editors=1112

this is a simple arithmetic test. in cases where it's necessary to create a closure object, the performance hit will be significantly higher - the for loop has zero memory overhead, whereas the functional version will require a closure object creation and subsequent garbage collection. in tight loops, like animation, this can have a noticeable effect.

[outofambit]

@Spongman to be clear, this is the use of a lambda as opposed to a for loop, which applies whether you use the es6 () => {} _or_ es5 function() {} syntax, correct? so i'm not sure its a big issue for this migration.

i don't see any issues in the branch @hsab shared above except potentially https://github.com/processing/p5.js/compare/master...es6#diff-ae09a6b88c1dd82501f4d0ea349ae764R31

[Spongman]

yes, this also applies to function(){} syntax.

i can't see that diff you linked to there, but there are definitely instances there where for loops have been changed to functional-style.

[hsab]

@Spongman just had a look at the code snippet you posted
I have 2 commits associated with for loops.
for-of:https://github.com/processing/p5.js/commit/739875749ed8383588a532ef1d078d16dd268417
forEach: https://github.com/processing/p5.js/commit/bef69ef9920d5e337f27ebd37b38eb24baa079fc

Luckily there are only 5 instances of forEach so reverting back to for is easy. But I was wondering if there is also a performance hit associated with for-of. Looking at your code it seems as if it performs slightly better than a for loop.

As for arrow functions, I was under the impression that performance is identical [[1](http://incaseofstairs.com/six-speed/)] [[2](https://stackoverflow.com/a/44031830)]

[Spongman]

Yes, it looks like for..of is now better than plain indexing, at least in the version of chrome I have here. It used to be worse, though.

Arrow functions and regular functions are the same, yes.

[hsab]

I will investigate the performance of for-of further with respect to other browsers.

If arrow functions have no performance hit, what did you mean by 'lambda functions' in your earlier comment? I was under the impression that they were synonymous in the ES6 lingo.

[limzykenneth]

Lambda functions is just another name for anonymous functions, it doesn't matter if it is using ES5 function() { } or ES6 () => { }, they are both lambda (anonymous) functions.

[Spongman]

Sorry I should have been more clear. The performance issues I brought up above apply equally to arrow functions and old-style functions. They should not be used to blindly replace for loops in library code - since doing so makes assumptions about the performance requirements of the client code.

[hsab]

Sorry I took "lambda functions/forEach" as "lambda functions and forEach" which lead to the confusion with the arrow functions and so on.

You're referring to the fact that 'forEach' requires a lambda function as a param and the associated overhead with each iteration. Makes perfect sense. Thanks for the clarification.

[lmccart]

hey all, this is a note to let everyone know we are planning to merge this ES6 update on wednesday morning pacific time. before then, we're trying to merge or close all outstanding PRs to reduce the number of merge conflicts. if you have a PR you are working on, please submit it in the next day or so if you can. otherwise, it would be best if you could hold until after the ES6 merge and rebase. thank you!

[lmccart]

I'm going to close this issue with the merge of #3874. thanks everyone for your thoughts! 🎉@hsab if there are any relevant design decisions in this thread not covered in your PR comments already, can you add these to the markdown file you're preparing?

[hsab]

Yes, thank you all for sharing your knowledge and expertise. So far I have learned a lot!

@lmccart Will definitely double check this thread before submitting the docs PR. Hopefully will be done by tomorrow afternoon.