Skip to content

Instantly share code, notes, and snippets.

@OliverJAsh

OliverJAsh/foo.md

Last active Aug 20, 2019
Embed
What would you like to do?
Dev UX: auto imports Dev UX: contextual naming Perf: tree shaking Dev UX: namespace/type merging
namespace import not yet yes maybe no
TS namespace yes yes no yes
types: namespace import / values: named import not yet / yes yes / no yes no
types: TS namespace / values: named import yes yes / no yes yes
@karol-majewski

This comment has been minimized.

Copy link

@karol-majewski karol-majewski commented Aug 19, 2019

What we want:

  • tree shaking
  • sweet syntax (type-namespace considered nice, fewer imports — like 1 — considered nice)
  • namespace type-merging?
  • no contextual naming? (context provided only once, don't be able to import checkIsFull without any context)
  • no Hungarian notation (no repeating Type or Entity inside the type name)
  • no naming conflicts (what do we rename?)
  • VSCode support (auto-imports, find references, rename symbol — asterisk imports/default exports)
  • Works with Unionized unions
@OliverJAsh

This comment has been minimized.

Copy link
Owner Author

@OliverJAsh OliverJAsh commented Aug 19, 2019

@karol-majewski

This comment has been minimized.

Copy link

@karol-majewski karol-majewski commented Aug 20, 2019

Final thoughts

  • Use the namespace keyword to aggregate types
  • Separate the API definition from application-specific types
  • Don't use namespace imports

Table of contents

Code sample

unsplash-web/src/services/Unsplash.ts
import { Unsplash } from "unsplash";
import { Collection, User, Photo } from "../entities";

/**
 * Type guards.
 */
function isCollection(candidate: any): Unsplash.Collection;
function isUser(candidate: any): Unsplash.User;
function isPhoto(candidate: any): Unsplash.Photo;

/**
 * Adapters
 */
function normalize(entity: Unsplash.Collection): Collection;
function normalize(entity: Unsplash.User): User;
function normalize(entity: Unsplash.Photo): Photo {}

/**
 * An HTTP client using the Unsplash API definition.
 */
enum Endpoints {
  Users = "/users/"
  User = "/user/:id"
}

export class Client {
  constructor(private baseUrl: string) {}

  get(endpoint: Endpoint.Users): Promise<User[]>;
  get(endpoint: Endpoint.User, id: string): Promise<User> {
    return fetch(endpoint).then(normalize);
  }
}
unsplash-web/src/side-effects.ts
import { Client as UnsplashClient } from "../services/Unsplash";

const unsplash = new UnsplashClient(process.env.UNSPLASH_API_URL);

unsplash
  .get("user", "q1w2e3r4")
  .then(validate)
  .then(synchronizeWithReduxStore);

Dos and don'ts

DO use the namespace keyword to aggregate types

// Bad
interface CollectionBasic {}
interface CollectionExtended {}

declare const myCollection: CollectionExtended;
// Good
namespace Collection {
  interface Basic {}
  interface Extended {}
}

declare const myCollection: Collection.Extended;
  • Easier to read
  • Hierarchies are clear

DO NOT use the namespace keyword to aggregate values

// Bad
namespace Collection {
  interface Basic {}
  interface Extended {}

  function isBasic(candidate: Collection): candidate is Collection.Basic {}
}
// Good
namespace Collection {
  interface Basic {}
  interface Extended {}
}

export function isBasic(collection: Collection): candidate is Collection.Basic {}
  • When a namespace contains values, the generated code includes bloated, non-tree-shakeable
    objects
  • Overloading identifiers in such a way is non-idiomatic and
    non-affordable. When you see a Collection, what can
    you do with it? Is it an interface? Is it a namespace? Is it a class? Should you call it,
    instantiate it, access its members?
  • Enforce with TSLint:"no-namespace": [true, "allow-declarations"]

DO separate your API schema from application-specific types

  • The API schema is unlikely to change. Application-specific entities are allowed to change. That's
    where you should put a boundary.
  • Conceptually, the relationship between the API and the client is one-directional. The client
    depends on the API. The API is agnostic towards its clients. Separating your API types from
    application-specific types reflects that relationship.
  • Easy to extract from this repository (to put it on npm, version, and share with others)
  • The API team can maintain it, deploy it, and guarantee its correctness
  • We don't know what the possible-feature-maybe-one-day SDK should consist of. We don't need to make
    that decision now. We can keep our helpers where they are right now — in our repository.

DO use declaration (d.ts) files to store your types

  • TypeScript doesn't allow executable code to live in declaration files
  • No risk of breaking tree-shaking by accidentally importing types and values via namespace import

DO use declaration merging to organize sibling types

// Bad
declare namespace Collection {
  interface Basic {}
  interface Extended {}
}

function isBasic(collection: Collection.Basic | Collection.Extended): collection is Collection.Basic;
// Better
declare namespace Collection {
  interface Basic {}
  interface Extended {}

  type Any = Basic | Extended;
}

function isBasic(collection: Collection.Any): collection is Collection.Basic;
// Best
declare namespace Collection {
  interface Basic {}
  interface Extended {}
}

type Collection = Collection.Basic | Collection.Extended;

function isBasic(collection: Collection): collection is Collection.Basic;
  • Easy to read
  • Doesn't need clumsy suffixes for disambiguation (like Collection.Type or Collection.Union)
  • Highlights the relationship (sibling variations of one type)

DO NOT pick a "default" entity when aggregating types

// Bad
declare namespace Collection {
  interface Basic {}
  interface Extended {}
}

type Collection = Collection.Basic;
// Good
declare namespace Collection {
  interface Basic {}
  interface Extended {}
}

type Collection = Collection.Basic | Collection.Extended;
  • It's more predictable if we do it the same way everywhere
  • Future proof — after all, you might change your mind as to what the "default" entity should be.

DO NOT use namespace imports

// Bad
import Collection from "some-library";
import * as Collections from "unsplash/lib/collections";

const collections: Collection<Collections.Collection> = [];
// Better
import Collection from "some-library";
import { Collection as UnsplashCollection } from "unsplash/lib/collections";

const collections: Collection<UnsplashCollection> = [];
// Best
import Collection from "some-library";
import { Unsplash } from "unsplash";

const collections: Collection<Unsplash.Collection> = [];
  • Namespace imports lead to inconsistent naming (because you can pick whatever name you want)
  • Named exports are more robust when it comes to tree shaking. Namespace imports create the risk of
    accidentally import types and values (see above)
    No longer the case?
  • Named exports always work with IDE features like renaming, importing and jumping to the definition.
  • Namespace imports lead to clumsy names like Collection.Collection and there is no way to reduce
    the amount of context. We should add no more context to a name than is necessary. To the contrary,
    named exports can be given more context if they don't have enough
    (import { Collection as UnsplashCollection } from 'unsplash').

Checklist

  • Third-party entities (here: Unsplash API entities) are pushed to the boundary of the application.
    • They are validated, normalized and purified there.
    • They don't spread.
  • All the requirements are met:
    • Tree shaking is guaranteed to work
    • The syntax is sweet
    • Contextual naming: there's never too much context. The amount of context can be increased
      when needed.
    • No clumsy suffixes like Type or Entity in the type name
    • No avoidable naming conflicts
    • Developer experience (auto-imports, find references, jump to definition, rename symbol)
    • Works with Unionize unions (?) (define works)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.