Improve docs around Record Types and classes

leebyron · leebyron · commit e65e5af806ea · 2018-05-18T11:29:31.000+02:00
Fixes #1495
diff --git a/__tests__/Record.ts b/__tests__/Record.ts
@@ -48,16 +48,16 @@ describe('Record', () => {
   it('setting an unknown key is a no-op', () => {
     const MyType = Record({ a: 1, b: 2, c: 3 });
 
-    const t1 = new MyType({ a: 10, b: 20 });
+    const t1 = MyType({ a: 10, b: 20 });
     const t2 = t1.set('d' as any, 4);
 
     expect(t2).toBe(t1);
   });
 
   it('falls back to default values when deleted or cleared', () => {
     const MyType = Record({ a: 1, b: 2, c: 3 });
-    const t1 = new MyType({ a: 10, b: 20 });
-    const t2 = new MyType({ b: 20 });
+    const t1 = MyType({ a: 10, b: 20 });
+    const t2 = MyType({ b: 20 });
     const t3 = t1.delete('a');
     const t4 = t3.clear();
 
@@ -68,13 +68,13 @@ describe('Record', () => {
 
     expect(t2.equals(t3)).toBe(true);
     expect(t2.equals(t4)).toBe(false);
-    expect(t4.equals(new MyType())).toBe(true);
+    expect(t4.equals(MyType())).toBe(true);
   });
 
   it('allows deletion of values deep within a tree', () => {
     const AType = Record({ a: 1 });
-    const BType = Record({ b: new AType({ a: 2 }) });
-    const t1 = new BType();
+    const BType = Record({ b: AType({ a: 2 }) });
+    const t1 = BType();
     const t2 = t1.deleteIn(['b', 'a']);
 
     expect(t1.get('b').get('a')).toBe(2);
@@ -90,7 +90,7 @@ describe('Record', () => {
 
   it('if compared against undefined or null should return false', () => {
     const MyType = Record({ a: 1, b: 2 });
-    const t1 = new MyType();
+    const t1 = MyType();
     expect(t1.equals(undefined)).toBeFalsy();
     expect(t1.equals(null)).toBeFalsy();
   });
@@ -115,7 +115,7 @@ describe('Record', () => {
   it('converts sequences to records', () => {
     const MyType = Record({ a: 1, b: 2, c: 3 });
     const seq = Seq({ a: 10, b: 20 });
-    const t = new MyType(seq);
+    const t = MyType(seq);
     expect(t.toObject()).toEqual({ a: 10, b: 20, c: 3 });
   });
 
@@ -129,7 +129,7 @@ describe('Record', () => {
   it('skips unknown keys', () => {
     const MyType = Record({ a: 1, b: 2 });
     const seq = Seq({ b: 20, c: 30 });
-    const t = new MyType(seq);
+    const t = MyType(seq);
 
     expect(t.get('a')).toEqual(1);
     expect(t.get('b')).toEqual(20);
@@ -138,18 +138,18 @@ describe('Record', () => {
 
   it('returns itself when setting identical values', () => {
     const MyType = Record({ a: 1, b: 2 });
-    const t1 = new MyType();
-    const t2 = new MyType({ a: 1 });
+    const t1 = MyType();
+    const t2 = MyType({ a: 1 });
     const t3 = t1.set('a', 1);
     const t4 = t2.set('a', 1);
     expect(t3).toBe(t1);
     expect(t4).toBe(t2);
   });
 
-  it('returns new record when setting new values', () => {
+  it('returns record when setting values', () => {
     const MyType = Record({ a: 1, b: 2 });
-    const t1 = new MyType();
-    const t2 = new MyType({ a: 1 });
+    const t1 = MyType();
+    const t2 = MyType({ a: 1 });
     const t3 = t1.set('a', 3);
     const t4 = t2.set('a', 3);
     expect(t3).not.toBe(t1);
@@ -158,7 +158,7 @@ describe('Record', () => {
 
   it('allows for readonly property access', () => {
     const MyType = Record({ a: 1, b: 'foo' });
-    const t1 = new MyType();
+    const t1 = MyType();
     const a: number = t1.a;
     const b: string = t1.b;
     expect(a).toEqual(1);
@@ -179,6 +179,7 @@ describe('Record', () => {
       }
     }
 
+    // Note: `new` is only used because of `class`
     const t1 = new ABClass({ a: 1 });
     const t2 = t1.setA(3);
     const t3 = t2.setB(10);
diff --git a/__tests__/RecordJS.js b/__tests__/RecordJS.js
@@ -8,10 +8,10 @@
 const { Record } = require('../');
 
 describe('Record', () => {
-  it('defines a constructor', () => {
+  it('defines a record factory', () => {
     const MyType = Record({ a: 1, b: 2, c: 3 });
 
-    const t = new MyType();
+    const t = MyType();
     const t2 = t.set('a', 10);
 
     expect(t.a).toBe(1);
@@ -44,6 +44,7 @@ describe('Record', () => {
       }
     }
 
+    // Note: `new` is only used because of `class`
     const t = new Alphabet();
     const t2 = t.set('b', 200);
 
diff --git a/type-definitions/Immutable.d.ts b/type-definitions/Immutable.d.ts
@@ -2229,17 +2229,19 @@ declare module Immutable {
    * myRecord.b = 5 // throws Error
    * ```
    *
-   * Record Classes can be extended as well, allowing for custom methods on your
+   * Record Types can be extended as well, allowing for custom methods on your
    * Record. This is not a common pattern in functional environments, but is in
    * many JS programs.
    *
-   * However Record Classes are more restricted than typical JavaScript classes.
+   * However Record Types are more restricted than typical JavaScript classes.
    * They do not use a class constructor, which also means they cannot use
    * class properties (since those are technically part of a constructor).
    *
-   * It's useful to think of Record Classes more like a Record Factory where
-   * record instances returned from the factory have additional API methods. It
-   * is best practice to not use `new` when creating new Records.
+   * While Record Types can be syntactically created with the JavaScript `class`
+   * form, the resulting Record function is actually a factory function, not a
+   * class constructor. Even though Record Types are not classes, JavaScript
+   * currently requires the use of `new` when creating new Record instances if
+   * they are defined as a `class`.
    *
    * ```
    * class ABRecord extends Record({ a: 1, b: 2 }) {
@@ -2248,7 +2250,7 @@ declare module Immutable {
    *   }
    * }
    *
-   * var myRecord = ABRecord({b: 3})
+   * var myRecord = new ABRecord({b: 3})
    * myRecord.getAB() // 4
    * ```
    *
diff --git a/type-definitions/tests/record.js b/type-definitions/tests/record.js
@@ -106,7 +106,9 @@ const originAlt2: TPointNew = MakePointNew();
 { const x: number = originAlt2.get('x') }
 { const x: number = originAlt2.x }
 
-// $ExpectError Use of new may only return a class instance, not a record
+// Use of new may only return a class instance, not a record
+// (supported but discouraged)
+// $ExpectError
 const mistakeOriginNew: PointNew = new MakePointNew();
 // An alternative type strategy is instance based
 const originNew: MakePointNew = new MakePointNew();

Original file line number	Diff line number	Diff line change
`@@ -2229,17 +2229,19 @@ declare module Immutable {`
`2229`	`2229`	`* myRecord.b = 5 // throws Error`
`2230`	`2230`	* ```
`2231`	`2231`	`*`
`2232`		`- * Record Classes can be extended as well, allowing for custom methods on your`
	`2232`	`+ * Record Types can be extended as well, allowing for custom methods on your`
`2233`	`2233`	`* Record. This is not a common pattern in functional environments, but is in`
`2234`	`2234`	`* many JS programs.`
`2235`	`2235`	`*`
`2236`		`- * However Record Classes are more restricted than typical JavaScript classes.`
	`2236`	`+ * However Record Types are more restricted than typical JavaScript classes.`
`2237`	`2237`	`* They do not use a class constructor, which also means they cannot use`
`2238`	`2238`	`* class properties (since those are technically part of a constructor).`
`2239`	`2239`	`*`
`2240`		`- * It's useful to think of Record Classes more like a Record Factory where`
`2241`		`- * record instances returned from the factory have additional API methods. It`
`2242`		- * is best practice to not use `new` when creating new Records.
	`2240`	+ * While Record Types can be syntactically created with the JavaScript `class`
	`2241`	`+ * form, the resulting Record function is actually a factory function, not a`
	`2242`	`+ * class constructor. Even though Record Types are not classes, JavaScript`
	`2243`	+ * currently requires the use of `new` when creating new Record instances if
	`2244`	+ * they are defined as a `class`.
`2243`	`2245`	`*`
`2244`	`2246`	* ```
`2245`	`2247`	`* class ABRecord extends Record({ a: 1, b: 2 }) {`
`@@ -2248,7 +2250,7 @@ declare module Immutable {`
`2248`	`2250`	`* }`
`2249`	`2251`	`* }`
`2250`	`2252`	`*`
`2251`		`- * var myRecord = ABRecord({b: 3})`
	`2253`	`+ * var myRecord = new ABRecord({b: 3})`
`2252`	`2254`	`* myRecord.getAB() // 4`
`2253`	`2255`	* ```
`2254`	`2256`	`*`