tuna icon indicating copy to clipboard operation
tuna copied to clipboard

Split code into separate files

Open Theodeus opened this issue 9 years ago • 27 comments
trafficstars

Theodeus avatar Sep 19 '16 13:09 Theodeus

Assigning this to you, @tencircles, let me know if you want a hand!

Theodeus avatar Sep 19 '16 13:09 Theodeus

I could help on this, if that would be helpful!

alexanderwallin avatar Dec 09 '16 10:12 alexanderwallin

Yes, that'd be great!

I think, as a first step, that we should stay away from using a module system (ES6 modules or what have you) and rather just split the files and add a concat npm script or something like that.

Moving to ES6 should be on the road map, but I'm not sure we should it all at once.

Am I crazy or do you guys agree? @tencircles & @alexanderwallin

Theodeus avatar Dec 09 '16 12:12 Theodeus

What is your main concern?

alexanderwallin avatar Dec 09 '16 12:12 alexanderwallin

Mainly doing the work in manageable chunks so that we don't get half way through and rage quit. :)

Could be, though, that switching to ES6 modules (which I personally prefer over CommonJS) would speed up the process rather than making it more complex?

Theodeus avatar Dec 12 '16 07:12 Theodeus

My bet's on the latter. :)

Why not create a branch, put an eslint config in there and we could start porting little by little?

alexanderwallin avatar Dec 12 '16 11:12 alexanderwallin

Maybe this fella can provide a kick start: https://github.com/lebab/lebab

alexanderwallin avatar Dec 12 '16 12:12 alexanderwallin

@Theodeus I've got some free time over the holiday. I can put in some hours splitting things up.

Honestly I was just thinking of doing something like this

Tuna.plugin(function module_name (dep1, dep2, dep3) {
    //...super cool audio effect goes here
});

or alternately

Tuna.plugin("SuperCoolPlugin", (dep1, dep2, dep3) => {
    //...super cool audio effect goes here
});

what do you think?

tencircles avatar Dec 20 '16 18:12 tencircles

here's a quick PoC

https://github.com/Theodeus/tuna/tree/banana

tencircles avatar Dec 21 '16 05:12 tencircles

What is the purpose of the plugin concept?

🍌

alexanderwallin avatar Dec 21 '16 11:12 alexanderwallin

to modularize the code without adding dependencies.

tencircles avatar Dec 21 '16 16:12 tencircles

So, audio worklets with next version of Chrome, and eventually a deprecation of the script processor. A refactoring has never been more urgent. :)

Should we adopt the class based approach used for worklets for everything?

https://developers.google.com/web/updates/2017/12/audio-worklet

Theodeus avatar Mar 22 '18 14:03 Theodeus

Oh, crap! It's been over a year since this thread was active. I was 100% certain it was this December we talked about this. 😮

Theodeus avatar Mar 22 '18 14:03 Theodeus

Would it be a good idea to let the actual nodes be the thing that you import? That way you don't get the whole forest when you ask for a banana. Each node imports the core Tuna object and register itself. So in the client code;

import {Chorus, Delay} from "tuna";
let myTunaNode = new Chorus(context);

In Chorus;

import Tuna from "./src/core";
Tuna.getInstance().registerNode("Chorus", _ => [implementation])

And finally in src/core;

 class Tuna {
     registerNode(name, node) {
        this._nodeTypes[name] = node;
    }
}

If you just need the Chorus node you'd probably save quite a few kb doing it this way. Whaddoyasay, @tencircles (and how do you feel about using classes)?

Theodeus avatar Apr 05 '18 18:04 Theodeus

As I'm posting this, I see the hole in my logic.. :)

Theodeus avatar Apr 05 '18 18:04 Theodeus

Two cents: I don't know the new API, but personally I would like a library like this to provide the tools to register all that myself, rather than solve everything for me. It's much harder to circumvent black box implementations than it is to copy-paste a recipe implementation.

alexanderwallin avatar Apr 05 '18 22:04 alexanderwallin

@alexanderwallin In that sense, do you think it would be better to have a bare-bones core of Tuna that you register the nodes you want to use to, or should it be as it is today, everything included, and if you want to add something extra you just add it to the Tuna prototype?

I've been out of js land for a while, so I'm not sure what the best way to go is. My concerns are these;

  1. Audio Worklets. Some refactoring needed to port from the script processors we use today, and the worklet format forces us to include separate files for each effect using them (as far as I can understand at least, haven't experimented with 'em yet). So one big tuna-min.js is probably not feasible in the future. Now, do we just leave it to the users to minify and so on and consider Tuna something that you have to integrate in your build pipeline, or can we make a "just grab it and include it in a script tag"-version as well? I'd think having a pre-built version is worth quite a lot.

  2. classes vs. prototypes. Every time i write the class keyword in js it hurts inside. :) But from what I can tell, classes has kind of won the battle in the community, and as I want Tuna to be as accessible as possible for everyone, it has some merit using classes as that is what's being preached everywhere.

Additionally, since I haven't spent time with worklets yet, a question I have is if it's supported to use a prototype based approach in creating those? The example I linked above explicitly states that a call to super() is required in an audio worklet (thus requiring a constructor/class approach). Reading the worklet discussions in the workgroup thread kind of enforces that perception. Does anyone know if the same thing can be achieved using prototypes directly?

Theodeus avatar Apr 06 '18 07:04 Theodeus

  1. To keep supporting all browsers, I guess we need to support both script processors and worklets, at least for a while..

Theodeus avatar Apr 06 '18 09:04 Theodeus

@Theodeus

Supporting both worklets and script processors would be tricky. I don't believe there is full browser support yet. Maybe we could host the worklet scripts on a CDN to save people the hassle of both excluding these files from their normal webpack/rollup build, and making sure they are served somewhere the worklet can access them. We can split out the ScriptProcessor code into separate modules, but I think we may want to hold of on worklet support until there is bigger adoption.

Regarding modules. None of the nodes have dependencies on the Tuna core, they really just need access to the user context. I would do something like the following.

// nodes/exampleNode.js
import TunaNode from "./TunaNode";

export default context => class ExampleNode extends TunaNode {
    constructor (settings) {
        this.node1 = context.createGain();
        this.x     = 2;
    }
};

// Tuna.js
import exampleNode from "nodes/exampleNode.js";
import otherNode from "nodes/otherNode.js";
// ...etc

export default class Tuna {
    constructor (context) {
        this.ExampleNode = exampleNode(context);
        this.OtherNode = otherNode(context);
        // ... etc
    }
}

Hopefully the pattern I'm suggesting is clear from the example, let me know if it's unclear.

Side note: I would highly recommend using rollup for bundling everything.

tencircles avatar Apr 06 '18 16:04 tencircles

Yeah, maybe I'm jumping the gun a little with worklets. I definitely agree that supporting both will be tricky. I guess the urgency will depend on how soon Chrome decides to deprecate (or, specifically, remove) the script processors. I don't think there's any public indication at all of worklets being implemented in safari, for instance, so we might end up with a need for both eventually.

That said, just focusing on modularisation for now is probably a healthy approach. :)

Theodeus avatar Apr 09 '18 06:04 Theodeus

Your example makes perfect sense, and rollup sounds good to me!

I still think it'd be a cleaner pattern if the nodes could register themselves with Tuna, so that we don't have to import and initialise each node type explicitly in Tuna.js. I kinda like it how it is today, that the nodes simply extend the Tuna prototype.

There should be an include-all-nodes version, just like tuna-min.js today, which is the main way to consume tuna (pun not intended).

But we should also include instructions on how to build Tuna from scratch, to allow those that want to shave kilobytes of the library to do so. I think that premise could help drive the choice of architecture. Do we want people, when building, to go into Tuna.js and remove the imports/init for the node types they don't need. Or do we want them to physically remove the individual node files to exclude them from the build?

I would assume that the tree shaking in rollup isn't clever enough to know that, for example, the Chorus node isn't used in an application using Tuna, if we import the Chorus in Tuna.js, right? If it is, then your approach sounds like the best way to go, and you can ignore the paragraph above.

So many questions. I should really try things out and experiment instead of expecting you to know everything. :)

Theodeus avatar Apr 09 '18 06:04 Theodeus

@Theodeus take a look in the split branch in the src directory. I started splitting things out. Not 100% on the approach, but it's a step in the right direction.

tencircles avatar Feb 13 '19 22:02 tencircles

Answering inline on the commit, but for visibility - I think I like it!

Theodeus avatar Feb 14 '19 11:02 Theodeus

@Theodeus I've been working on this PR. Part of the reason it's taking so long is the AudioParam mock. I think this is quite necessary actually, but it requires a bit of legwork to implement. I'll hopefully have an update on this soon.

tencircles avatar Aug 12 '19 14:08 tencircles

Sounds awesome! I'd be happy to assist if you need a second pair of eyes/hands. :)

Theodeus avatar Aug 12 '19 14:08 Theodeus

@tencircles How is your progress? We are waiting for it. Can I see your changes work in progress? (I may help you :) )

kaibadash avatar Sep 10 '20 08:09 kaibadash