search

ipld / team-mgmt

IPLD Team Planning, Management & Coordination threads
6 stars 9 forks source link

JavaScript patterns and practices #81

Closed mikeal closed 1 year ago

mikeal commented 4 years ago

For the most part, decisions about patterns and toolchains have been left to individual maintainers in IPLD. This was working well enough because there weren’t very large divergences in preferences between team members.

However, upgrading to ESM has caused some bigger fractures than previous differences. This change in particular forces additional pattern changes on consumers and so it’s important to pull back a little and discuss what patterns and practices we should be aligned on in the team and which ones can continue to be left to individual maintainers.

The JavaScript ecosystem has seen a lot of churn in recent years. Features have landed in the language and been adopted on various timelines in the ecosystem. Even though there hasn’t been an alignment between systems programmers (Node.js only) and browser programmers on defaults it has been relatively easy to find compatibility even between large changes like callbacks to async await.

One thing has been observably true: libraries that stay with older patterns are eventually factored out of active libraries, replaced by alternatives that adopt new patterns.

While it often seems like “sticking with what is working” is without cost, that is not always true. Taking a future breaking change will leave a fair amount of your ecosystem bound to older patterns and it gets harder to justify these kinds of changes the more you’re adopted. Deferring hard decisions like this only makes them harder in the long run.

Given that our adoption is modest and most of what we’re working on is new libraries it makes sense that we would optimize for future adoption. We don’t have a large and entrenched ecosystem that we need to worry about maintaining compatibility with. We have the opportunity to adopt new patterns, even when it causes issues for people still using older patterns, because we can optimize for the future. We’re not at a stage of maturity where it’s even advantageous for us to appeal so broadly, we’re actually more interested in being adopted by developers who are comfortable on the bleeding edge because that’s the state of our software.

ESM is going to be a harder break than prior JavaScript improvements we’ve seen. We continue to find seams and even cracks in the compatibility between modules as you vary ESM/CJS and various bundlers and versions of Node.js. But that doesn’t mean this transition isn’t going to happen, it just means that the penalties for not upgrading to newer patterns might end up being larger than prior conversions like callbacks to promises.

There’s also a growing list of new toolchains that either greatly prefer or flat out only work with ESM (rollup, snowpack, etc). These are high growth tools with high growth ecosystems around them that I’m more worried about being cut out of than I am appealing to people who can’t or won’t be adopting new patterns over the next few years.

Something I need to keep in mind is that I’m far too comfortable seeing and even tearing at the seams of the platforms we’re building on. I’m actually quite used to finding bugs like we’re finding in ESM and either fixing them directly or getting them to whoever will be able to fix them. This isn’t something we can expect our users and even early contributors to do.

I’m not that concerned about things that don’t work too nicely when you aren’t writing ESM because I think it’s easy for us and our early users and contributors to use ESM. But I am worried about issues we’ve seen and found in V8’s native coverage because issues like this can’t be fixed by us and we have to find workarounds that regular developers won’t easily figure out.

Those are my thoughts and I’d like to hear what others think. My recommendations for team wide practices going forward are:

Which gives us:

And these are the tradeoffs we’d accept.

rvagg commented 4 years ago

What are the implications for our biggest consumer, js-ipfs? Is this likely to mean that js-ipfs will perpetually be stuck on older generation IPLD and it'll take investment in a new thing (lite, whatever) to move away from it? Or is there a realistic path to seeing js-ipfs make a similar shift? They just went through a painful async/await migration, are they likely to opt-in to this too? One implication of this being that we're not going to be able to retire/archive the old IPLD stack any time soon, so we're effectively increasing our support surface area.

Right now I'm doing CAR import/export to js-ipfs and I can't use any of the new stack, so I'm not going to merge anything in js-datastore-car that touches js-multiforamts, or at least if I do I'll have to maintain two branches of it so that js-ipfs isn't polluted either by this pattern or by a very large collection of dependencies only brought in to support this specific feature.

olizilla commented 4 years ago

What are the implications for our biggest consumer, js-ipfs?

I second this emotion. Does...

Use the rollup config script I built for CJS support in Node.js

...mean this could be done without breaking changes for current consumers like js-ipfs, who have limited bandwidth to take on extensive refactors.

vmx commented 4 years ago

I'm in favour of ESM, but I don't want to break the world (i.e. the js-ipfs ecosystem).

The current consume-ESM-as-CJS story is just not there. To me breaking the current webpack version (v4) is not an option.

I think we should only adopt ESM now, if we find a way to use ESM for Multiformats/IPLD stuff, with zero impact for CJS consumers.

We had to many refactors like ipfs-block -> ipld-block, which took a long time (1 person week or so), with zero benefits for any user/dev. There were other cases like that before I can't recall ATM. I really don't want to see that happen again at a yet larger scale.

mikeal commented 4 years ago

I’m working through some of the implications, but it looks like this isn’t a “wait and it’ll get better” situation. Webpack has been defending this bug for years and is still shipping it, so this is a break in the ecosystem that everyone migrating to ESM will have to contend with.

That said, there’s ways to work around it and to mitigate it and we’re still building tooling that will improve that.

It’s premature to talk about js-ipfs adopting our new stack, we’re not even done updating our own code to it and I would expect smaller/newer users like js-ipfs-lite to adopt it before js-ipfs as well. We’ll learn a lot in that process and should have acceptable solutions to these compatibility concerns.

Honestly, this is probably not the thing you’ll spend the most time dealing with migrating to. Switching to Uint8Array and the new CID interface are much harder because they are value types that get passed around all over the place, swapping import and export statements is relatively easy by comparison.

I’m going to be in Monday’s IPFS Core Implementations call to discuss in more detail.

mikeal commented 4 years ago

Oh ya, one thing I should mention, we already wrote a compatibility layer so that new codec work doesn’t fork away from what js-ipfs can consume.

https://github.com/multiformats/js-multiformats/blob/master/legacy.js

We can take any new-style codec and get an interface in the old ipld-format style and it’ll swap Uint8Array for Buffer and switch the CID implementations. I wrote this because I figured it would be a while before js-ipfs would be able to migrate and I didn’t want us to have to manage two implementations of every codec during that time.

All that’s left to do is to build and publish CJS-only packages of the old style interfaces as part of our automated releases. In the meantime we have a separate repo for both dag-cbor implementations until we write the build tooling and sufficiently test it.

rvagg commented 4 years ago

It’s premature to talk about js-ipfs adopting our new stack

Well, the CAR file export thing is one example, another one might be to integrate modern BTC, Zcash and ETH work that I'm doing which is entirely based around the new multiformats stack. There's increasing interest in getting coin explorers working (again) for the web. All of that is either off the table for now, or I have to wind the code changes back a little to use the old stack. I still haven't merged those PRs and I'm pondering the best approach here since buying into ESM seems so very costly right now if this code is to be relevant to anything existing and not cause a spiralling upgrade cycle for anything that wants to consume it.

One thing that bothers me about the webpack v4 thing that maybe someone else can answer (who cares to understand more about webpack than I do), why isn't this solved as an opt-in plugin since their config-heavy approach allows for all sorts of extension points? Surely there's a loader that people that want to consume export default the sane way can slot into their config and be done with it? If that were the case, I'd stick that in Polendina by default, I'd also be more comfortable jumping on this bandwagon and just telling users "if you want to consume this with Webpack 4 then you need to use this plugin or else it won't work". Or maybe this stuff is so deeply fundamental to Webpack that it can't be worked around.

rvagg commented 4 years ago

An alternative is to just avoid default exports entirely, write it off as an unusable pattern for the next couple of years. We lose some elegance and make for awkward naming (create()), but could bypass the particular class of problems we're running in to at the moment. Node 10 will still be a problem but that problem is slowly fading.

Gozala commented 4 years ago

I wonder how and if deno support had being considered ? It would be a shame if even after switching to Uint8Arrays and ESM things did not work on deno out of the box.

I would also like to float idea of adopting https://prettier.io/ as you're at it. In my experience https://standardjs.com/ is nice but it's rules still do not fully eliminate ambiguity and leave room for opinions, furthermore ESLint base tooling isn't always capable to format code without me manually doing things (although it definitely had improved over the past few years)

mikeal commented 4 years ago

@Gozala I have some code running in Deno now, and as a result I’m a bit more skeptical on what the timeline might be for our stack to work on it OOTB.

Sure, we’ve mostly shed our dependence on Node.js stdlib and are on ESM, but it’s not very practical right now to run on Deno unless you have no dependencies whatsoever. That includes having dependencies between our own libraries. The solution to this problem just isn’t entirely worked out yet in Deno. I’m confident this will get better over time, but I wouldn’t hold my breath for our stack to work in the near term.

I could probably get js-multiformats w/o the legacy API running given a little time, because there’s almost no dependencies, but for all the libraries up the stack that depend on that module, they don’t have a great way to depend on it and import it reliably that can be published in a cross-platform way.

mikeal commented 4 years ago

another one might be to integrate modern BTC, Zcash and ETH work that I'm doing which is entirely based around the new multiformats stack

This is what I wrote the legacy interface for. We can take these new-style codecs and get the old style interface with everything turned back to Buffer and the old CID and js-ipfs can use them without taking any of our breaking changes.

Gozala commented 4 years ago

I could probably get js-multiformats w/o the legacy API running given a little time, because there’s almost no dependencies, but for all the libraries up the stack that depend on that module, they don’t have a great way to depend on it and import it reliably that can be published in a cross-platform way.

I was mostly thinking about js-multiformats as a way to pave that path. I’m suspecting lot of browser specific things I’m doing could also be made to work in deno if js-multiformats could. E.g I’m doing some web File stuff that I’ve end up implementing for node & potentially could have version for deno

rvagg commented 4 years ago

This is what I wrote the legacy interface for. We can take these new-style codecs and get the old style interface with everything turned back to Buffer and the old CID and js-ipfs can use them without taking any of our breaking changes.

This doesn't solve the Webpack problems though, right? As soon as js-multiformats gets included somewhere in a project that's using webpack@4 (or 5 even I suppose) we run into this same default exports problem. I don't think legacy solves any of that and this is the problem I'm mostly concerned about now. Legacy helps with Buffer and old-CID, but not with ESM which is the focus of this discussion.