use the alea npm module rather than embedding. fixes #31

[jwagner]

A quick update on this, I'm still torn between adding alea as a dependency, keeping it in the module or removing it completely. They all have their own pros and cons. I want to give an in between approach a shot. For that I'd refactor the code so it exports the individual noise functions directly something like simplexNoise2D(random: () => number) => (x: number, y: number) => number; I'd then keep then probably keep the built in alea for the existing 'class based' faced.

That should result in minimal bundle sizes (and dependencies) for advanced users and the same old 'class base faced' for backwards compatibility and ease of use.

[mreinstein]

still torn between adding alea as a dependency, keeping it in the module or removing it completely.

I don't understand why adding alea as a dep is controversial; the baked in implementation is byte-for-byte the exact thing that's in the npm published alea module. It's not going to affect build size or code quality, and for projects that have more than one module using alea it means we can eliminate deps that are getting inlined in.

Maybe there really is some eng tradeoff that I'm not seeing? What are the downsides to using the alea module as published on npm?

[jwagner]

the baked in implementation is byte-for-byte the exact thing that's in the npm published alea module.

I don't think that's true. At least if we are talking about this one: https://github.com/coverslide/node-alea/blob/master/alea.js There is an UMD wrapper, state imports, exports, support for 'fract53' etc. At least from naive numbers just running the code through uglifyjs the builtin variant is ~ 457 bytes, the linked one 1092 bytes.

Maybe there really is some eng tradeoff that I'm not seeing? What are the downsides to using the alea module as published on npm?

One more dependency, one more person to trust regarding supply chain attacks or just simple mistakes, unless the version is locked down. If it is locked, the potential of still ending up with a duplicate. Slightly more overhead on npm install.

All of those are marginal issues, but factoring out the alea module is also about marginal gains.

[mreinstein]

one more person to trust regarding supply chain attacks or just simple mistakes, unless the version is locked down

It is locked down; once something is published to npm that version number and it's associated module content is immutable. As long as the package-lock.json file in simplex-noise is updated when the deps change.

There is an UMD wrapper, state imports, exports, support for 'fract53' etc. At least from naive numbers just running the code through uglifyjs the builtin variant is ~ 457 bytes, the linked one 1092 bytes.

Fair point, there is more stuff in the alea module. All good reasons to try to get them to improve the alea module. Modernizing and simplifying that helps everyone. :) If that author is unresponsive or disagrees then we could publish our own alea, and assuming it's better I would switch over to it in all of my other alea dependent projects as well.

[mreinstein]

https://github.com/coverslide/node-alea/issues/8

[jwagner]

It is locked down; once something is published to npm that version number and it's associated module content is immutable. As long as the package-lock.json file in simplex-noise is updated when the deps change.

That is true for simplex-noise itself but as far as I know it won't hold for applications that depend on it. Since you specified "alea": "^1.0.0", in this MR, if someone installs simplex-noise he'll get the latest 1.x of alea. Not the one in the packet-lock.json of simplex-noise. If the developer of alea were to have a bad day, all of the simplex-noise users might be having a bad day as well. As far as I know the package-lock is only relevant to the top level package (i.e. the one in which npm install is running). That could be changed by requiring an exact version ("1.0.0"), nut then users might get duplicates again if they (or another library they use) requires a newer version.

In my opinion the technically right solution to this is to invert/remove the dependency and just allow users to pass in any PRNG function that satisfies () => number since there really isn't anything special about alea in this case. Any decent PRNG will do for building the permutation tables.

If I recall correctly I've added alea in to make simplex-noise easier to use since I've got a few questions regarding how it can be used with a seed even though there was an example in the readme. At least from memory I haven't received any questions about seeding it since so I guess adding alea directly solved that.

In retrospect that might have been misguided and maybe I should have just improved the docs instead.

[mreinstein]

Not the one in the packet-lock.json of simplex-noise

That's because simplex-noise does not currently include a package-lock.json file in it's versions. If we were to add that file the exact deps would indeed be locked at whatever is listed in simplex-noise's package-lock.json at it's time of creation.

If the developer of alea were to have a bad day, all of the simplex-noise users might be having a bad day as well.

The same could be said of simplex-noise today, even without any deps. On some level it boils down to philosophy; is it worth having any re-use in a module ecosystem? I don't think that because there are some bad modules/actors out there that we should throw out all re-use with it. I think it's more about curating dependencies carefully, and alea has so far shown itself to be responsibly maintained.

In my opinion the technically right solution to this is to invert/remove the dependency and just allow users to pass in any PRNG function that satisfies () => number since there really isn't anything special about alea in this case.

I agree that is an elegant solution, though it does change the API of simplex-noise in a big way. I was proposing alea as a dep because it could be achieved with a patch or minor release. As long as you're ok with a more dramatic change that bumps major, it will work (that would be ok with me, happy to pass alea as an arg everywhere.)

btw thanks for the participation! It's nice to see there are some module authors out there that still maintain their bits. ❤️

[jwagner]

That's because simplex-noise does not currently include a package-lock.json file in it's versions.

So if I'd add 'package-lock.json' to the files list in package.json that behavior would change? I wasn't aware of that. Do you know where that is documented? Would love to read up on it.

The same could be said of simplex-noise today, even without any deps. On some level it boils down to philosophy; is it worth having any re-use in a module ecosystem? I don't think that because there are some bad modules/actors out there that we should throw out all re-use with it. I think it's more about curating dependencies carefully, and alea has so far shown itself to be responsibly maintained.

I totally agree, it's a minor issue.

I agree that is an elegant solution, though it does change the API of simplex-noise in a big way.

Jep, on the plus side it could be made tree shakeable in the process.

btw thanks for the participation! It's nice to see there are some module authors out there that still maintain their bits.

That goes both ways. I'm very happy to see that there are people engaging with this little lib, even if I'm terribly slow to reply at times. :)

[mreinstein]

Would love to read up on it.

1st bullet under the description section here: https://docs.npmjs.com/cli/v8/configuring-npm/package-lock-json#description

[mreinstein]

I can close this in favor of removing alea entirely, if that is still the plan

[jwagner]

@mreinstein I'd suggest to leave it open until we have decided which way to go. I'm hacking on the tree shakeable version now, I'll create a PR for that as well and collect some feedback on it.