Un-deprecate

[JamieB-gu]

Why?

As described in #136, we planned to migrate everything in this repo to @guardian/libs. Several features have already been moved across (thanks @jamie-lynch!): guardian/libs#212, guardian/libs#220, guardian/libs#221, guardian/libs#222

Unfortunately, we've been unable to migrate everything - for more details see guardian/libs#193. As a result, these features will have to stay in @guardian/types so they can be consumed by the projects that use them, receive updates and bug fixes etc.. Therefore we're not in a position to deprecate the project for now 😔.

Changes

• Remove deprecation warning from README

[jamie-lynch]

Could we instead add a note to the effect of this PR description to give any new adopters a heads up that, unless they need any of the functionality that isn't going to be migrated, they should be using libs?

[JamieB-gu]

I'm assuming that we'll be deleting the features that have been migrated? So the only things left will be the ones that haven't?

[SiAdcock]

Hi @JamieB-gu

I want to clarify what we mean by deprecated, as it might help alleviate some of your concerns.

The plan is for @guardian/types to no longer receive new feature updates or changes to its API. Client Side Tools and Infra recommend that you migrate to using @guardian/libs for most of the features that were defined here.

Option and Result will not be supported for reasons that have previously been discussed. We recommend that teams find an alternative solution, and we are more than happy to help with this.

We'll keep the deprecation notice in place because we don't want teams to start using this library or the patterns and types within. We won't accept any new PRs except security fixes. We won't un-publish this library or delete any features. We will continue to monitor usage of this library to ensure it is on a downward trajectory.

Please let me know if there's anything I can help with, if anything is unclear or if I have said anything that is unacceptable.

[JamieB-gu]

Thanks for attempting to clear things up @SiAdcock 🙂

Option and Result will not be supported for reasons that have previously been discussed. We recommend that teams find an alternative solution, and we are more than happy to help with this.

We'll keep the deprecation notice in place because we don't want teams to start using this library or the patterns and types within. We won't accept any new PRs except security fixes. We won't un-publish this library or delete any features. We will continue to monitor usage of this library to ensure it is on a downward trajectory.

Couple things on this. Firstly, I don't think that's feasible as a universal rule I'm afraid. As mentioned on that thread, Option and Result are already in use in a few codebases (some of them extensively), and they're working well for us. I don't think we'd want to phase them out, or even stop supporting them with new features. Secondly, I'm struggling to join the dots from the "Coding With Empathy" recommendation to the removal of these features. Reading through that document I don't see anything that specifically relates to this discussion 🤔 - however I may well be missing something, would you mind clarifying?

My understanding was that you'd like @guardian/libs to act as a foundational framework, providing recommendations for client-side patterns you'd like to see adopted universally across the department. I also get that you're not keen on Option and Result, and that not every client-side developer would like to work with them. Therefore, as much as I'd like to see them in libs, I get why we've chosen not to move them in and I think that's reasonable.

However, I'm afraid that I don't accept the premise that we should be prescriptive to the point where we mandate that no team should be using these software patterns regardless of whether they find them helpful. Some developers do like working with these features for a number of reasons (which I've discussed elsewhere).

At the risk of drifting into the abstract here: I get that it's valuable to have consistency across codebases, and there are a lot of scenarios where I argue in favour of that. But I also think that if we take this too far we're in danger of a) stifling innovation by cutting off the exploration of new techniques we can use to improve our projects, and b) making developers lives more difficult by denying them the tools that they like to work with. Bringing this back to something a bit more concrete: for me personally, I want to write the best, most robust code I can, and I don't want the tools that allow me to do that to be taken away from me 🛠😔.

[SiAdcock]

Hi @JamieB-gu. I don't have much time today, but the crux of your concerns appears to this:

I don't accept the premise that we should be prescriptive to the point where we mandate that no team should be using these software patterns regardless of whether they find them helpful

I don't think that was what I was suggesting, apologies if it was unclear. I'm suggesting you find an alternative solution to consuming them from this library, which is deprecated. I can help you find alternative ways of consuming these patterns if you would like.

I believe the relevant point from Coding with Empathy is:

prefer idiomatic patterns and techniques
this will optimise our code for people who are already familiar with the language it is written in

The argument is that Option and Result are not typically familiar to TypeScript developers, they are idioms imported from other languages. As such, they make code that uses them harder to grok.

[sndrs]

Secondly, I'm struggling to join the dots from the "Coding With Empathy" recommendation to the removal of these features. Reading through that document I don't see anything that specifically relates to this discussion 🤔 - however I may well be missing something, would you mind clarifying?

Given the following, I'd say:

prefer idiomatic patterns and techniques (this will optimise our code for people who are already familiar with the language it is written in)

Option/Result are not idiomatic to JavaScript/Typescript (if they were/became so then I think there would be nothing to discuss).

prefer simplicity over sophistication (this will optimise our code for the next reader)

They're an alien abstraction with a bespoke implementation – that can't be simpler than writing native typescript code (concision is not the same as simplicity).

defer to the principle of least surprise (our code should make the reader feel capable rather than out of their depth)

You could be fluent with typescript and have never seen Option/Result before. What would be least surprising to you then?

But I also think that if we take this too far we're in danger of a) stifling innovation by cutting off the exploration of new techniques we can use to improve our projects,

Killing off innovation is 100% not what we should be doing and in this situation something we've thought hard about. you've clearly put an awful lot of time and thought into this and that alone is extremely welcome; anything that kills that, even a bit, is massively undesirable.

But that can be true while also not being a sufficient acceptance criteria.

Something that challenges our assumptions or how we work is a good thing, and trying these things out is vital if we want to progress. but testing them means understanding we might decide against them (the collective ‘we’).

Concluding that a new approach will make too many of our lives harder to adopt it globally is a reasonable response – dismissing all change is not, but also not what i think is happening here.

and b) making developers lives more difficult by denying them the tools that they like to work with.

What should we do for developers whose lives are made more difficult by non-idiomatic approaches denying them the fluency they have spent time acquiring?

Bringing this back to something a bit more concrete: for me personally, I want to write the best, most robust code I can, and I don't want the tools that allow me to do that to be taken away from me 🛠😔.

I think it's worth remembering that our job is to build and maintain good products. Writing code is how we do much of it, but writing code is not the point of it.

When we do write code, it needs to support the creation and maintenance of our products by whoever works on them and at any time. We all work in the context of our colleagues. This means not only writing code that is likely to work, but also that is the most likely to be understood by the most number of people.

It's clear that this approach is how you feel most able to do the former, but we're trying to balance that here against the latter. That is ultimately what I think we all should be doing.

[JamieB-gu]

Thanks both for your comments 🙂

I don't think that was what I was suggesting, apologies if it was unclear. I'm suggesting you find an alternative solution to consuming them from this library, which is deprecated. I can help you find alternative ways of consuming these patterns if you would like.

Awesome, that makes sense, thanks Simon. As discussed offline, I think we've clarified that this is more an issue with the types package having a name that's far too generic, and your concerns that CSIT feels they have to maintain it? As mentioned, happy with the idea of renaming/moving to a new repo to solve this problem 👍.

Option/Result are not idiomatic to JavaScript/Typescript

I'm afraid I don't agree with that Alex, particularly in the case of TypeScript. I think this is where we run into the issue I raised when this recommendation was introduced a few weeks ago. Determining whether something is "idiomatic" is subjective, particularly in the world of JavaScript, and it's hard to resolve this without a definition of idiomatic.

All that said, I would find it helpful to understand why these don't seem idiomatic from your perspective. For me these two types are just made up of features that are common in the TypeScript world, like enums and generics, and techniques that are well-documented in the TypeScript handbook, like discriminated unions. Furthermore the entire API consists of a few short functions, which are a foundational feature of JavaScript. Does this seem like a fair description, or do you see things differently?

They're an alien abstraction

I'm not sure I understand this phrase, could you clarify? Do you mean they're an abstraction that comes from somewhere else?

that can't be simpler than writing native typescript code (concision is not the same as simplicity)

I agree with the second part of this - the idea that concision is not the same as simplicity. I've seen lots of examples in multiple languages (JavaScript, Scala, Python etc.) where something is concise but very hard to reason about. However, these weren't introduced for the purpose of concision, and I'm not actually sure they are more concise than other solutions in any case. They were introduced because I don't believe the first part of this statement is always true. I don't think the native JS/TS techniques are particularly simple or easy to reason about. As I've mentioned before, in a lot of cases I think they're quite concise but hide a lot of complexity - we've discussed examples of this in a few places now. The data types discussed here were introduced to reduce complexity and make the code more robust (i.e. fewer bugs).

Killing off innovation is 100% not what we should be doing and in this situation something we've thought hard about. you've clearly put an awful lot of time and thought into this and that alone is extremely welcome; anything that kills that, even a bit, is massively undesirable.

I appreciate this, thank you 🙂

trying these things out is vital if we want to progress. but testing them means understanding we might decide against them (the collective ‘we’).

Are you saying that the collective "we" has both tested these data types and decided against using them?

Concluding that a new approach will make too many of our lives harder to adopt it globally is a reasonable response

I thought we'd already decided we weren't going to adopt these globally when we concluded that they weren't going to live in libs?

What should we do for developers whose lives are made more difficult by non-idiomatic approaches denying them the fluency they have spent time acquiring?

I've covered the topic of "non-idiomatic" above, but is there an implicit assumption here that it will make people's lives more difficult? How many people have actually tested this approach? How do we know it's more difficult than dealing with the many problems that stem from JavaScript existing "nullable"/error-handling? How do we know it's not easier? It's only been battle-tested in a few applications so far, and it's been used specifically in AR for nearly two years now and refined during that time.

I think it's worth remembering that our job is to build and maintain good products. Writing code is how we do much of it, but writing code is not the point of it.

I agree. Introducing these isn't an excuse to play around with new programming concepts. They've been added because they make our production code more robust by avoiding bugs caused by the complexity of JavaScript's native approach, as mentioned. That's a win for the product but also for developers.

When we do write code, it needs to support the creation and maintenance of our products by whoever works on them and at any time. We all work in the context of our colleagues. This means not only writing code that is likely to work, but also that is the most likely to be understood by the most number of people.

It's clear that this approach is how you feel most able to do the former, but we're trying to balance that here against the latter.

I also agree with this. But again, I don't think this approach contradicts that. I've tried very hard to make the biggest impact with the smallest overhead for new developers. We've not really added anything beyond the Option and Result types that are common to many languages, and the API has remained minimal. It's also why I'm reluctant to jump to fp-ts, as I've discussed with both you and Simon before now, as I think that will introduce a much larger conceptual surface area. Also, we have experimented with adding other features before now, but feedback from the team suggested this was a bit much, so we didn't go with it. The point I'm trying to make is that I think the balance you mentioned is in evidence here.

On a related topic, you've got to understand that not everybody who works on our codebases is a JavaScript expert, and it's a difficult language to work with even for people who consider their competency with it to be good. There are a lot of gotchas and corner-cases that you just have to remember because they don't occur in other languages. One thing I'm trying to do is sand off some of the rough edges and make it easier to contribute for people who have to work across multiple stacks and languages. You mentioned that these concepts feel "alien", but my point is that they're actually common to many languages and JavaScript is the outlier here. From my perspective it's simpler to learn something once and be able to apply it in many environments, rather than have to learn a collection of specific concepts that have no parallels elsewhere.