pbomb / flow-immutable-models Goto Github PK

Generates model classes from Flow types using Immutable.js

JavaScript 100.00%

flow-immutable-models's Introduction

flow-immutable-models

This repository contains a codemod script for use with JSCodeshift that creates model classes backed by Immutable.js data structures based on Flow type aliases.

Motivation

Immutable collections are great for simplifying application development by avoid object mutations and enabling performance optimizations such as memoization and reference equality comparisons. A popular immutable collection library is Immutable.js.

One downside to using Immutable.js collections (Immutable.List, Immutable.Map, etc.) is that the objects do not lend themselves to static analysis / typing with tools like Flow or TypeScript. For example, with Flow we often end up typing Maps like Immutable.Map<string, any>; This means that the map contains unknown keys of type string values can be of any kind. It says nothing about which keys are allowed and what type a value for a given key should be. While there are some ways of providing better typing than this, there are still gaps in how well these objects can be described.

This codemod library takes the approach of wrapping an Immutable.Map with a typed ES6 class definition. As a consumer, you would create files with exported Flow type definitions described as an Object with defined keys and values. Running this codemod against these files creates an ES6 class with getters and setters for each typed property. Each setter function returns a new instance of the class so that you can continue to take advantage of performance optimizations like memoization and reference equality checking since the class instances are immutable.

Getting Started

Follow these steps to install this library as a dependency in your application.

If using yarn

yarn add flow-immutable-models

If using npm

npm install --save flow-immutable-models

Executing the codemod script

jscodeshift -t node_modules/flow-immutable-models/lib/transform.js <path>... [options]

Use the -d option for a dry-run and use -p to print the output for comparison. For more information about the jscodeshift CLI options, check out its README.

How it works

This codemod modifies any file that exports Flow type declarations named like *ModelType. For each matching exported Flow type, a model class will be created later in the file. If this script is re-run and the model class already exists, it will be updated to reflect any changes to the describing ModelType, meaning it is safe to run this script multiple times against the same files.

It's also possible to nest ModelTypes together or to define properties to be collections. The way to do this is to describe the ModelType purely as JS Objects and Arrays and the library will create model classes that will convert the plain-JS objects into Immutable.js collections as necessary.

For more information, please read through the various recipes, starting with the Basic one, to see how it works.

Recipes

CLI Options

Options to recast's printer can be provided through the printOptions command line argument

jscodeshift -t transform.js <path> --printOptions='{ "quote":"double" }'

The default options are

{ "quote": "single": "trailingComma": true }

flow-immutable-models's People

Contributors

Stargazers

Watchers

Forkers

object88 heedster thorsten-plause rgbkrk ddouglascarr dperetti mayank1791989 dharanbro

flow-immutable-models's Issues

Clarify the specs regarding extra properties in objects passed to fromJS()

When we instantiate a model, it's critical to make sure not to use properties that are not part of the model type, especially because it can be the result of a typo.

For example, given the definition:

export type BarModelType = {
  barStr: string,
  barNum: number,
};

This should yield an error:

const bar = Bar.fromJS({ barstr: "blah" })

because barstr !== barStr.

It's not clear whether the library is supposed to allow Flow to detect these errors.
Currently, sometimes it does not, and sometimes is does but it seems to be a side effect of the use of $Shape<> in some condition (when default properties are defined).
I'm not 100% sure about this, but I think this requirement (no extra properties) could be precisely enforced by prepending $Shape<BarModelType> & ... to the json parameter of the fromJS(). method.

Option to separate model from generated file

Hi! Really great work on this codemod, @rgbkrk and I have had fun experimenting with it!

One thing we're worried about is the discoverability of our code after generating the model file. We see typing as a way to provide documentation for newcomers joining the project, but the current output after running the codemod feels a bit intimidating.

Would it be possible to decouple the models from the generated classes, maybe by moving them into their own file?

Document & test default values

The README currently doesn't describe how to use default values and we don't currently have any tests including default values.

Work around Flow issue with Object.assign

Based on facebook/flow#2683, we should be able to remove the $FlowFixMe comment generated in the fromJS function.

ES5 lib

I think this should be lib/ImmutableModel.js. I don't want to transpile external packages.

Detect when ModelType is erroneously applied to union type

I've been experimenting with using this codemod on nteract's core notebook model (which uses commutable underneath).

The base repo I've set up for experimenting with is https://github.com/nteract/commutable-models, which you can try out with flow-immutable-models by running:

git clone https://github.com/nteract/commutable-models
cd commutable-models
npm i
npm run generate:models

I was getting this error

> [email protected] generate:models /Users/kylek/code/src/github.com/nteract/commutable-models
> jscodeshift -t ./node_modules/flow-immutable-models/lib/transform.js ./src/models.js

Processing 1 files...
Spawning 1 workers...
Sending 1 files to free worker...
 ERR ./src/models.js Transformation error
TypeError: Cannot read property 'filter' of undefined
    at fromJS (/Users/kylek/code/src/github.com/nteract/commutable-models/node_modules/flow-immutable-models/lib/helpers/fromJS.js:66:57)
    at makeClass (/Users/kylek/code/src/github.com/nteract/commutable-models/node_modules/flow-immutable-models/lib/transform.js:21:47)
    at NodePath.<anonymous> (/Users/kylek/code/src/github.com/nteract/commutable-models/node_modules/flow-immutable-models/lib/transform.js:81:17)
    at __paths.forEach (/Users/kylek/code/src/github.com/nteract/commutable-models/node_modules/jscodeshift/dist/Collection.js:76:36)
    at Array.forEach (native)
    at Collection.forEach (/Users/kylek/code/src/github.com/nteract/commutable-models/node_modules/jscodeshift/dist/Collection.js:75:18)
    at exports.default (/Users/kylek/code/src/github.com/nteract/commutable-models/node_modules/flow-immutable-models/lib/transform.js:68:6)
All done.
Results:
1 errors
0 unmodified
0 skipped
0 ok

Which was a result of erroneously putting the ModelType suffix on a union type like this:

export type CellModelType = CodeCellModelType | MarkdownCellModelType

when it should have been

export type Cell = CodeCellModelType | MarkdownCellModelType

PR that fixed it: nteract/commutable-models#2

At least for development experience, it would be nice if this package could detect this type of behavior. Right now, if propExpressions is undefined in fromJS it's likely that this is the case.

Automatically include necessary imports if missing from a model file

The current code requires that ImmutableModel must exist in a model file for it to be functional. It would be nice to include that import in the generated portion of the file if it is missing.

Change setters to have return type of `this`

Instead of setters returning the class name, use this so that the class can be extended. To support this, switch setters from calling new ClassName to using this.clone.

I'm not sure about whether extending model classes will let users sleep well at night, but this change itself doesn't seem harmful.

Provide `toJS` and `toJSON` functions

In order to support serialization, provide a toJS (and toJSON alias) function to convert the ImmutableModel into its modelType form.

Allow reference model type field to be maybe type

When defining a model type with a property of another model type like this:

export type MyModelType {
  field: OtherModelType,
};

the fromJS function for MyModelType calls the fromJS from the Other model class. However, if field is defined as a maybe type (e.g. field: ?OtherModelType), the codemod doesn't recognize it as a model type or do anything in the fromJS function.

The correct behavior would be to still have the fromJS function contain code like the following, taking the null/undefined case into account:

  state.field = state.field == null ? state.field : Other.fromJS(state.field);

Generated fromJS can omit state variable declaration

Given model...

export type BazModelType = {
  bazNum: number,
  innerBazs: Array<InnerBazModelType>,
};

export type InnerBazModelType = {
  key: string,
  value: string,
}

... the codegen produces a fromJS for Baz which doesn't declare and initialize state:

static fromJS(json: BazModelType): Baz {
  state.innerBazs = state.innerBazs.map(item => InnerBaz.fromJS(item));
  return new Baz(Immutable.fromJS(json));
}

Proposal: instead of only creating a state variable if there is a default[ModelType]Values, always do so. Thoughts?

Babel Transform

Since you've gone to the trouble of writing JSCodeShift transforms, have you considered using a babel transform instead? They're actually quite similar.

Using babel would make it easier to transparently tweak the underlying implementation without requiring a re-run of the codemods.

Just an idea -- very interesting project! 🙇

Support static properties

When a type has a static string like cell_type below here:

export type MarkdownCellType = {
  cell_type: 'markdown',
  source: string,
}

export type CodeCellType = {
  cell_type: 'code',
  source: string,
  outputs: Immutable.List<string>,
}

export type Cell = MarkdownCellType | CodeCellType;

It would be great if the generated class set cell_type as a static property on the generated class.

/cc @peggyrayzis

Construct model using Immutable.Map instead of Immutable.fromJS

Immutable.from can be too heavy-handed, especially since we're handling nested models by explicitly calling the other model's fromJS method.

Extend Immutable.Record instead of composing with Immutable.Map

Explore extending Immutable.Record with same getters / setters. With this approach, we might be able to add in default values (initialValues variable) so we can be more sure about types not having to be maybe types.

Support for IntersectionType

Any plans or thoughts on supporting IntersectionType ?
I run in to use cases always where my swagger api models are often inherited from something else. But as of now, since the lib only supports ObjectType, I have to repeat all the properties in each of the type again in the inherited model type.

It would be great if we can support something like

type B = A & {
   bar: string
}

where A is an existing ModelType,
so B can be a class that extends A instead of ImmutableModel.

Add comments to generated source

Indicate that the class has been generated and users should not modify it.

Convert Objects as Maps fields into corresponding Immutable.Map

Allow model types to be defined with fields typed as Object as Maps and convert them into Immutable.Maps. For instance, a model type defined like this:

export type MyModelType = {
  field: { [key: string]: number },
};

would create code like the following.

fromJS

state.field = Immutable.Map(state.field);

getter

get field(): Immutable.Map<string, number> {
  return this._state.get('field');
}

setter

setField(field: Immutable.Map<string, number>): My {
  return new My(this._state.set('field', field));
}