Skip to content

Instantly share code, notes, and snippets.

@TylerK
Last active March 6, 2019 21:03
Show Gist options
  • Save TylerK/f5fbf4178348b32029cc6901e1241943 to your computer and use it in GitHub Desktop.
Save TylerK/f5fbf4178348b32029cc6901e1241943 to your computer and use it in GitHub Desktop.
Fleet App Next™

Fleet App Next™

Table of Contents

Goals

Short term

  • Rename the current working branch to fleet-app-next and start accepting PR's there.
  • Discuss current work and remaining items with whomever wants to help.
  • Test and launch this work ASAP

Medium term

  • Get the modules PR up and running
  • Start converting all existing UI to the new modules format
  • Build out Trips 2.0 and other new UI features

Long term

  • Move towards a GraphQL architecture

Dependencies

It's been almost two years since we settled on a lot of our versions and dependencies. It's time to update these to take advantage of some of the new cool toys out there.

  • TypeScript: Bump to 3.x
    • Too many great features to list here. See: 1, 2, 3, 4
  • React: Bump to 16.8
    • Hooks
    • Improved context API
    • Async rendering performance
  • Yarn: Bump to 1.14.x
    • Better monorepo support
    • Faster, better, stronger
    • Tons of bug fixes since 1.10 (our current version)

There are likely a few other libraries we would like to upgrade, I just haven't done a thourough deep dive in our package.json yet.

back to top 🔺

Modules

Overview

Kevin has done some great work around getting us setup to move to a far more modular UI for our front end applications. Follow along here: https://github.com/nauto/web-apps/pull/924

Reasons for doing this work

  • Decouple and blackbox as much of the UI as possible for maximum re-usability and testabilty
  • ui-kit has grown out of control, and requires testing and publishing everything anytime we need to make minor changes to anything
  • Semvar will make sense again if applied to small, isolated functions
    • Like seriously, what does 2.12.5 even mean in terms of a massive collection of disparate components?
  • Allow us to use Lerna to it's full potential
  • Greatly speeds up change time for small fixes
  • Spin up new projects with greater ease
  • As long as the module outputs React and or es-modules, we have greater flexibility to expiriment with new methods, ie: React Hooks.

What does a module look like?

modules
  └─ widget
    ├─ lib
      ├─ index.d.ts
      └─ index.js
    ├─ src
      ├─ locales                    // All necessary strings
        ├─ en-US
          └─ translation.json
        ├─ ja-JP
          └─ translation.json
      ├─ index.tsx                  // Entry point file
      ├─ index.mdx                  // docz file for UI components
      └─ index.test.tsx             // Entry point unit tests
    ├─ .babelrc
    ├─ i18next.config.js
    ├─ jest.config.js
    ├─ tsconfig.json
    ├─ tslint.json
    ├─ rollup.config.js
    ├─ README.md
    └─ package.json

That is a lot of boilerplate, for sure. However this allows us to build and publish discrete pieces of well tested and documented UI.

Sunsetting UIKit as a repository for UI components

Soon we will be breaking ui-kit down into major modules and seperating them out into individually versioned, namespaced NPM modules.

Before

import Widget from '@nauto/uikit/dist/components/widget';

After

import { Widget } from '@nauto/widget';

Docz

We will start replacing all Storybook files with Docz files. I have a branch where I've done a ton of this work already so we've got a nice headstart here. I believe Docz offers much nicer features, and a better end user experience for developing UI. .mdx files are infinitely nicer to work with than .story.js files.

Check out: https://www.docz.site/

New UIKit project

A new UIKit project will become the public face for our UI. This new project will scan the modules folder for mdx files and correlate them into a single style guide. Since docz allows developers to create themes for displaying their UI, it shouldn't take very long to build a Nauto branded styleguide.

I would love to see this project deployed to https://ui.nauto.systems.

Modular i18n

Problem

All issues with POEditor and our current process aside, we are loading about 36kb (gzipped) of JSON into the browser all at once for each language on inital page load.

Solution

As we move to a more modular arcitecture it stands to reason we should also lazyload translations with each module. This will help to greatly speed up initial page loads.

back to top 🔺

Moving Away From Decorators

I believe we should start making use of components that expose render props, in lieu of relying on our current HoC solution. This will make our code more declarative and allow us to make use of stateless components in future work.

Before

import withFeatureFlags from '@nauto/feature-flags';
import Widget from '@nauto/widget';

@withFeatureFlags
class Foo extends React.Component {
  render() {
    const { flags } = this.props;
    return flags.showWidget ? <Widget /> : null;
  }
}

After

import Feature from '@nauto/feature-flags';
import Widget from '@nauto/widget';

class Foo extends React.Component {
  render() {
    return <Feature flags={flag => flag.showWidget && <Widget />} />;
  }
}

But why not function as a child?

This write up is pretty succinct

back to top 🔺

A Nicer API Interface

Current

import { config } from '@nauto/config-file';
import { path } from '@nauto/utils/path';
import { getFleetId } from '@nauto/utils/fleets';
import { fetchRequest } from '@nauto/utils/request';

const url = path(`${config.api_url}/fleets/${getFleetId()}/events/${endpoint}`, params);

return fetchRequest(url)
  .then(...)
  .catch(...)

Problems:

  • config.api_url is a constant that is imported and repeated in every redux file
  • getFleetId() is also a constant that gets imported and repeated in every redux file
  • Requires a lot of imports from disparate util files to build up a URL string and make a request
  • None of this is typed, even a little bit

Proposal #1

import { match, request } from '@nauto/api';

/**
 * the url function could accept an object
 * to return a fully formed URL:
 * https://api.nauto.systems/v2.2/fleets/f-n2-internal/events/drivers?foo=bar
 */
const url = match({
  path: `fleets/:fleetId/events/${endPoint}`,
  params: {
    foo: 'bar',
  }
});

/**
 * Or a string to return a fully formed URL:
 * returns: "https://api.nauto.systems/v2.2/fleets/f-n2-internal/events"
 */
const url = match('/fleets/:fleetId/events');

return request(url)
  .then(...)
  .catch(...)

"request" would default to get, in order to facilitate other operations we would add methods to the request class, and additionally take an optional data object.

request.post(url, data);
request.put(url, data);
request.delete(url, data);

Proposal #2

Utilizing a fully typed and documented API class.

import { api, request } from '@nauto/api';

// ...

return request(api.fleet.events.drivers)
  .then(...)
  .catch(...)

This would have the benefit of fully abstracting away URL building from the calling function, as well as enabling intellisense.

@ahgroner
Copy link

Case 1 (about 40/60 use cases of featureFlag in our current code)
use a feature flag for a show/hide in JSX

{featureFlags.flagName && <feature>}

OR

{featureFlags.flagName ? <feature> : <other>}

Above code totally makes sense for this case and is way cleaner.

Case 2 (about 20/60 use cases in our current code)
use featureFlags in lifecycle functions to make a business logic determination
(For example, call action A or action B based on the flag, or pass the flag as a parameter into a function)

So where we previously did

<Parent>
    <ChildWithFeatureFlagLogic /> // @withFeatureFlags decorated
</Parent>

without decorators we would now do:

<Parent>
      <Feature flags={({ showWidget }) => (
       <ChildWithFeatureFlagLogic flagLogic={flag}  />
      )} />
</Parent>

Again, this is for the case where we need the flag outside of render JSX.

What about Case2 but for a .route file. Unsure the best way to wrap that component in component. Gets a little cumbersome without a decorator. But if there's a good solution open to it.

@ahgroner
Copy link

An alternative implementation for Case 2 would be changing the flow inside ChildWithFeatureFlagLogic

So what used to be:

@withFeatureFlags
class ChildWithFeatureFlagLogic {
   handler = () => {
      const { featureFlags: { flag } } = this.props
      // do something based on the flag
   }
   render (
      // stuff...
      <div onClick={handler} >
   )
}

becomes:

class ChildWithFeatureFlagLogic {
   handler = (flag) => {
      // do something based on the flag
   }
   render (
      // stuff...
          <Feature flags={({ flag }) => (
             <div onClick={handler.bind(null, flag)}  />
          )} />
   )
}

This seems like it would end up being hard to trace, we're passing the flag inside render instead of props so it's less top-down.
I probably prefer the code in my last comment.

@ahgroner
Copy link

ahgroner commented Feb 28, 2019

As a last thought - in addition to the Feature component I'd LOVE an even simpler version of the that can be written as just

      <Flagged show={"flagName"} > 
         <Widget />
      </Flagged>

would be pretty easy to build, just renders children if said featureFlag is true. And can type check against IFeatureFlags for safety.

Just having the nested JSX and closing /Flagged tag would make our components more readable vs.

   <Feature flags={({ flagName }) => (
      flagName && <Widget />
   )} />

which tends to trip us up in the context of larger JSX blocks.
Also, less risk of messing up truthiness in the flagName && part.

(this whole comment is a nit, and something that could be built later as a quality of life improvement)

@TylerK
Copy link
Author

TylerK commented Feb 28, 2019

All of these issues are solved by wrapping the component at a higher level

@TylerK
Copy link
Author

TylerK commented Feb 28, 2019

<Flagged show={"flagName"} > 
  <Widget />
</Flagged>

Clean but a non-starter. This requires maintaining a map between flags and strings, and disallows any branching logic to rendering it's children or passing data if needed.

@alunov-equinix
Copy link

alunov-equinix commented Mar 1, 2019

Sounds like a good plan!
Like this changes 👍

Regarding Adam's last thought.
So we might combine both variations to have two props: show as string and flags as function.

Clean but a non-starter. This requires maintaining a map between flags and strings.

Object destructuring for "render prop" case ({ flagName }) is similar to flags[${flagName}]. So we just need to have proper obejct properties.

So for 60% of feature-flag code we can use this aproach with show prop as string, to have easily readable, clean structure:

   <Feature show="flagName" > 
     <Widget />
   </Feature>

And for other feature-flag code use "render prop" variant with flags prop as render function.
Thus we can pass down flag prop and use it in lifecycle methods:

   <Feature flags={({ flagName }) => (
      <Widget flag={flagName} />
   )} />

Or in case of some additional branching logic:

   <Feature flags={({ flagName }) => (
     flagName ? <WidgetA /> : <WidgetB />
   )} />

@alunov-equinix
Copy link

alunov-equinix commented Mar 1, 2019

It might be a bit different topic but would be good to organize z-index layers.
There were some bugs related to incorrect z-index layering.
Not to have z-index: 99999; and to prevent same in the future we might need some z-index module/file to regulate it across all styles, similar to what we have for sizes in Ui-kit:

const zIndexLayers = {
  min: 1,
  lower: 2,
  medium: 3, 
  upper: 4,
  higher: 5,
  max: 6,
};

@TylerK
Copy link
Author

TylerK commented Mar 1, 2019

I'm down for a zIndex enum!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment