Skip to content
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 some missing class. #6173

Merged
merged 15 commits into from
Jun 12, 2023
Merged

Conversation

asukaminato0721
Copy link
Contributor

Resolves #3758

More class to es6, this is a continue to #6075

Changes:

I use p5\..*?.prototype\..*? to find them.

Screenshots of the change:

PR Checklist

@asukaminato0721
Copy link
Contributor Author

@asukaminato0721
Copy link
Contributor Author

I think now it's ok to merge 👀

@limzykenneth limzykenneth merged commit cf06ad0 into processing:main Jun 12, 2023
@asukaminato0721 asukaminato0721 deleted the missing-es6 branch June 13, 2023 04:48
@jeffscottward
Copy link

I'm late to the conversion party but just wondering why use a classes?

Definitely agree we have to get away from prototype but at least in the react.js (react team moved away after using it) and node.js worlds, classes are frowned upon. It's a jarring context switch for a language that is designed to be functional.

Is using classes required for this switch away from prototype?

@davepagurek
Copy link
Contributor

Is using classes required for this switch away from prototype?

I would say that it's not required, but is the easiest change, since the previous code was already using classes but just using an ES5 syntax for it, so we could change to class syntax without updating any call sites.

@jeffscottward
Copy link

Is using classes required for this switch away from prototype?

I would say that it's not required, but is the easiest change, since the previous code was already using classes but just using an ES5 syntax for it, so we could change to class syntax without updating any call sites.

Oh that is smart actually. Ok fair. :) I might try to throw AI at this and see if it can blaze through.

@jeffscottward
Copy link

Is using classes required for this switch away from prototype?

I would say that it's not required, but is the easiest change, since the previous code was already using classes but just using an ES5 syntax for it, so we could change to class syntax without updating any call sites.

Oh that is smart actually. Ok fair. :) I might try to throw AI at this and see if it can blaze through.

Uh oh..
https://github.com/processing/p5.js/blob/main/contributor_docs/archive/es6-adoption.md#browser-compatibility-and-transpiling

http://incaseofstairs.com/six-speed/

Assuming we are using Babel - Classes is a massive perf hit. 8x? wow.
What about typescript?

Also - based on Contrib docs - seems like a master Issue for ES6 conversion is missing? These one of PR's leave the conversation sporadic in closed issue comments. Should I make one?

@jeffscottward
Copy link

🚨 🚨 Please move ES6 conversion commentary over to #6371 🚨 🚨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ES6 Transition: Concerns & Progress
4 participants