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

Port remaining files completely to ES6+ syntax and modules #2629

Closed
26 of 28 tasks
meganindya opened this issue Nov 28, 2020 · 54 comments
Closed
26 of 28 tasks

Port remaining files completely to ES6+ syntax and modules #2629

meganindya opened this issue Nov 28, 2020 · 54 comments
Labels
Good first issue Issue-Architecture Requires lots of refactoring (maybe across a large number of files) Priority-Major Important, but program still runs

Comments

@meganindya
Copy link
Member

meganindya commented Nov 28, 2020

Most of the JavaScript files contain code a lot of ES5 code, that better be updated to ES6 (and above, e.g. async/await). It's not like we should be bothered about ceding any further browser support because the codebase already contains ES8 elements.

As for the ES5-ES6 syntax, we should port to the following things:

  • var to let/const
  • arrow functions in callbacks
  • Object prototype definition to class definition (consider use of static members too)
  • for ... in/for ... of loops wherever appropriate

In addition, replace function declaration within functions with const function variable declarations. Consider effects of hoisting.

Replace

...
function foo( ... ) {
    ...
    function bar( ... ) {
        ...
    }
    ...
}
...

with

...
function foo( ... ) {
    ...
    const bar = ( ... ) => {
        ...
    }
    ...
}
...

Port to class definition and more:

  • block.js
  • blockfactory.js
  • blocks.js
  • boundary.js
  • languagebox.js
  • palette.js
  • pastebox.js
  • pluginsviewer.js
  • protoblocks.js
  • saveInterface.js
  • toolbar.js
  • trash.js
  • widgets/help.js
  • widgets/meterwidget.js
  • widgets/modewidget.js
  • widgets/musickeyboard.js
  • widgets/oscilliscope.js
  • widgets/phrasemaker.js (rename class from PitchTimeMatrix to PhraseMaker)
  • widgets/pitchdrummatrix.js
  • widgets/pitchslider.js
  • widgets/pitchstaircase.js
  • widgets/rhythmruler.js
  • widgets/statistics.js
  • widgets/status.js
  • widgets/temperament.js
  • widgets/tempo.js
  • widgets/timbre.js
  • widgets/widgetWindows.js

Even though the js/blocks/ API uses classes. Most of the code needs cleanup/refactoring. A lot of the classes aren't even used (their corresponding blocks have been deprecated). Proper identification and documentation can possibly be helpful when we port the blocks. Do not remove any code though. Unused variables can be gotten rid of.

@meganindya meganindya added Good first issue Issue-Architecture Requires lots of refactoring (maybe across a large number of files) Issue-Enhancement Priority-Major Important, but program still runs labels Nov 28, 2020
@meganindya
Copy link
Member Author

meganindya commented Nov 28, 2020

This need not be done all at once, rather it must be done incrementally lest the changes should be too invasive.

I'll start working with js/activity.js. Probably most other files can be dealt with in any order.

@kushal-khare-official
Copy link
Contributor

@meganindya I would like to help with this too.

I can start with porting to class definition.

@meganindya
Copy link
Member Author

Pls go ahead. Maybe start with the smaller ones, e.g. widgets. Be careful though.

Add me as a reviewer on the PR (better one file at a time).

@ricknjacky
Copy link
Member

@meganindya sounds great!

As for the ES5-ES6 syntax, we should port to the following things:

  • var to let/const
  • arrow functions in callbacks
  • Object prototype definition to class definition (consider use of static members too)

I am starting off by porting var to let/const. Thankyou for the articulate explanation of what needs to be done.

@meganindya
Copy link
Member Author

One important advice: NEVER DO A REPLACE ALL WHEN THERE ARE A LOT OF CASES. We have had trouble before. Hopefully, you're aware about the scoping issue with var. There are a lot of redeclarations in many places. Please be careful.

@ricknjacky
Copy link
Member

I have started porting
currently I am working on timbre.js file
I have kept scoping in mind, as you have rightly pointed out in the previous comment
still @meganindya I'd prefer you reviewing it, and correct me, if there's any discrepancy

@sj00coder
Copy link
Contributor

I would I like to help with this,I will change callbacks to arrow functions. @meganindya

@endurance21
Copy link

@meganindya i think there is only one /blocks.js that has not been covered yet?
This file seems to be big in size approx 6k lines , is there any other way to distribute and then work on it ?

@dhruv-doshi
Copy link

@meganindya i think there is only one /blocks.js that has not been covered yet?
This file seems to be big in size approx 6k lines , is there any other way to distribute and then work on it ?

I'm working on blocks.js.
If needed, we can split

@endurance21
Copy link

@dhruv-doshi cool !
Let me know if I can help in some way.👍✨

@meganindya
Copy link
Member Author

@endurance21 perhaps you can try your hand at #2632.

@endurance21
Copy link

Thanks @meganindya

@dhruv-doshi
Copy link

Hey @meganindya

I was looking at #2637 and the changes seem to be similar to what I have been editing and the old PR was merged. I would like to know what was the change that has to be made.

Thanks

@meganindya
Copy link
Member Author

I can't seem to figure where those changes went. The code is still the old one. Weird!

Anyway, if you have a more tried and tested version of the ES6 port, please create a PR. Otherwise, I could copy the changes from #2637.

@dhruv-doshi
Copy link

I would love to contribute the code. It shall be a learning experience for me.

@meganindya
Copy link
Member Author

Go ahead.

@souptik4572
Copy link

I would love to contribute by converting the functions to ES6 format.

@JanitChawla
Copy link

Hey @meganindya
Is there any changes left to made would love to contribute

@KaranSingh1301
Copy link
Contributor

hello @meganindya
js>activity.js is not mention in the checkbox but still have eslint error and do not have class components. Can I start with it?

And sir as you mention about MB.2 , will it be have a new structure of code or addon over existing architect of musicblocks. Any of the framework is also be taken into consideration like React?

@Ayush-Khandelwal-007
Copy link

I would like to work on the remaining files @meganindya. Should I start working on it?

@vikasazad
Copy link

Hi @meganindya I also wanted to a helping hand in this can you tell me where to begin

@soumyo123-prog
Copy link

Hi, @meganindya please assign me a function which is available. I am new to contributing in sugarlabs

@samyak-max
Copy link

Hey, @meganindya I'm a newbie and would love to contribute.

@meganindya
Copy link
Member Author

Music Blocks is being built from the ground-up, to address several architectural problems with this run. Since Music Blocks is a fork of Turtle Blocks JS, musical functionality was added on top of it. However, music is fundamental to Music Blocks. Besides, the Turtle Blocks JS started initially with handful of features and was written without a complex architecture. As Music Blocks got built on top of that, it got incrementally complex, but the architecture remained simple, thus resulting in a monolith. Also, the functionality is tightly coupled with the interface and native client API (Web API).

Keeping these problems in mind, we have considered a foundational rebuild that will address all these issues, whilst adding buffers for future additions. We'll also be using a more elegant tech-stack to develop and maintain this project given its scale.

Refer to the repository sugarlabs/musicblocks-v4 for more information about the new project — Music Blocks (v4).

@aritroCoder
Copy link

If this is open, I can work on this

@ManavSarkar
Copy link
Contributor

Hi, The /js/blocks.js file doesn't have the class definition, can I work on it?

@meganindya
Copy link
Member Author

Please see #2629 (comment)

@ManavSarkar
Copy link
Contributor

ManavSarkar commented Dec 15, 2022

Please see #2629 (comment)

There are some files which I have ported today only to ES6+ syntax and modules such as js/block.js and other files. Are they ported already or they are left? If they are not ported, shall I send a PR? @meganindya

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good first issue Issue-Architecture Requires lots of refactoring (maybe across a large number of files) Priority-Major Important, but program still runs
Projects
None yet
Development

No branches or pull requests