Feature/3.0.0 #56

geclos · 2019-05-17T16:06:20Z

WAT

src/Collection.ts

src/types.d.ts

package.json

src/Model.ts

README.md

package.json

Co-Authored-By: Fran Casas <nflamel@gmail.com>

franciscoj · 2019-05-21T13:24:59Z

__tests__/Base.spec.ts

+          expect(requestError).toBeInstanceOf(ErrorObject)
+          expect(requestError.payload).toEqual({ foo: 'bar' })
+          expect(requestError.error).toEqual(null)
+          expect(requestError.requestResponse).toEqual(null)


what would happen if the exception isn't raised?

I don't understand. We are explicitly raising that exception, it can't not* be raised. It would be raised with empty values in the worst-case scenario.

👌 I'm just worried that in case something made that code correct, the test would just pass.

masylum · 2019-05-21T14:08:36Z

src/Collection.ts

-  model (attributes: { [key: string]: any } = {}): Class<*> {
-    return Model
-  }
+  abstract model (attributes?: { [key: string]: any }): new(attributes?: {[key: string]: any}) => T;


👀 was is das?

model is an abstract method returning a generic T class constructor that should be defined in child extending this parent class.

omg, javascript syntax is getting out of hand haha

src/Collection.ts

masylum · 2019-05-21T14:13:35Z

src/Model.ts

@@ -210,7 +200,6 @@ export default class Model extends Base {
      .then(data => {
        this.set(data)
        this.commitChanges()
-        return data


any reason for this change?

There's no reason for it unless there's a concatenated then statement.

const promise = new Promise((resolve, reject) => { resolve('payload') }) promise.then(payload => { return 'another payload' }) promise.then(payload => console.log(payload)) // output => 'payload'

I'm not sure I understand the question. Do we want to keep the promise returning the response or not? This is a backward incompatible change I understand.

It is not a backward incompatible change. The promise resolves with the HTTP response to all its hooked then statements, regardless of the return statements of each particular then callback.

This is what I'm trying to explain in the piece of code above ☝️ , notice that the second then statement receives the original promise resolution result, and not the returned value of the first then statement.

I think I see. I'm not sure the complexity added with withRequest is worth it. I think this is actually also the reason why the switch from async/await to raw promises (can't attach twice a then just using await).

masylum · 2019-05-21T14:14:36Z

src/Model.ts

@@ -284,15 +277,16 @@ export default class Model extends Base {
            this.set(this.applyPatchChanges(data, changes))
          }
        })
-
-        return data


masylum · 2019-05-21T14:15:04Z

src/Model.ts

+      if (optimistic && this.request) this.request.progress = progress
+    })
+
+    const { promise, abort } = apiClient()[method](this.url(), data, { onProgress, ...otherOptions })

    promise


why are we using promises now instead of async/await?

It's been like this since the 3.0.0 alpha PR. Do not want to introduce more entropy in this PR.

masylum · 2019-05-21T14:15:50Z

src/Model.ts

@@ -239,7 +228,7 @@ export default class Model extends Base {
  @action
  save (
    attributes?: {},
-    { optimistic = true, patch = false, keepChanges = true, ...otherOptions }: SaveOptions = {}
+    { optimistic = true, patch = false, keepChanges = false, ...otherOptions }: SaveOptions = {}


what is this keepChanges flag?

This was introduced in the 3.0.0 alpha PR. From description:

Saving keeps changes that were made after the request started but before it finished (can be disabled by passing the option keepChanges = false).

And you reverted the change in this commit.

masylum and others added 16 commits August 20, 2018 00:09

The road to 3.0

2410794

More items

79995b7

Update flow and fix tests

976a6b5

Reimplement mustGet and mustFind

604ae0a

reimplemented mustGet and mustFind methods

3cfe81b

Added progress for latest request

1285bbf

Updated changelog

e69d1cc

Update CHANGELOG.md

f5bee17

Changed RPC signature to provide custom label as an optional value

09231a6

OCD

c64b977

added documentation for missing methods

657ac9d

removed unnecessary return values in some promises

4f9b0e4

Attempt to fix #47

26f1397

Accept a rootUrl in rpc endpoints. fix #41.

b2aacff

Remove flow #1

103bb83

Migrate to typescript 🎉

61b1d46

geclos changed the title ~~Feature/3.0.0.beta~~ [DO NOT MERGE][WIP]Feature/3.0.0.beta May 17, 2019

updated changelog

2b2252c

franciscoj reviewed May 20, 2019

View reviewed changes

src/Collection.ts Outdated Show resolved Hide resolved

franciscoj reviewed May 20, 2019

View reviewed changes

src/types.d.ts Outdated Show resolved Hide resolved

geclos force-pushed the feature/3.0.0.beta branch from 05c59f0 to fea98e1 Compare May 20, 2019 10:56

Migrated tests to Typescript

03fd8cb

geclos force-pushed the feature/3.0.0.beta branch from fea98e1 to 03fd8cb Compare May 20, 2019 10:59

geclos added 2 commits May 20, 2019 13:04

updated travis config

b15ee5a

added abstract declaration to map method

3107ae2

geclos force-pushed the feature/3.0.0.beta branch from 065cfe1 to 3107ae2 Compare May 20, 2019 11:13

added some scripts

7106bb3

geclos force-pushed the feature/3.0.0.beta branch from 60b60cc to 7106bb3 Compare May 20, 2019 11:21

franciscoj reviewed May 20, 2019

View reviewed changes

package.json Outdated Show resolved Hide resolved

franciscoj reviewed May 20, 2019

View reviewed changes

src/Model.ts Outdated Show resolved Hide resolved

franciscoj reviewed May 20, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

franciscoj reviewed May 20, 2019

View reviewed changes

package.json Outdated Show resolved Hide resolved

geclos and others added 6 commits May 20, 2019 15:28

target ES5

02a4a1a

Update README.md

0c9f165

Co-Authored-By: Fran Casas <nflamel@gmail.com>

tweak README

a389d82

bump version!

931f51f

bundle type declarations with package

1f794b0

more README improvements

43d448c

geclos changed the title ~~[DO NOT MERGE][WIP]Feature/3.0.0.beta~~ Feature/3.0.0.beta May 20, 2019

udpate CHANGELOG

94ecd88

geclos changed the title ~~Feature/3.0.0.beta~~ Feature/3.0.0 May 20, 2019

geclos force-pushed the feature/3.0.0.beta branch from 85a3485 to 94ecd88 Compare May 20, 2019 14:39

geclos added 2 commits May 20, 2019 17:04

updated npmignore

cf2b5e2

Include payload as a fallback in case of API error

6fbb666

geclos force-pushed the feature/3.0.0.beta branch from 84ccc45 to 6fbb666 Compare May 21, 2019 11:21

add custom paylaod to ErrorObject

311a933

franciscoj reviewed May 21, 2019

View reviewed changes

masylum reviewed May 21, 2019

View reviewed changes

src/Collection.ts Outdated Show resolved Hide resolved

masylum reviewed May 21, 2019

View reviewed changes

geclos added 4 commits May 21, 2019 18:10

add() returns array of added models

0e0f2e8

removed TODOs

0911748

Defend against requesting patch during a POST operation

969fc69

Updated CHANGELOG

2a5c2db

geclos force-pushed the feature/3.0.0.beta branch from b96cd53 to 2a5c2db Compare May 29, 2019 13:36

geclos merged commit 0bf782b into master May 31, 2019

geclos deleted the feature/3.0.0.beta branch May 31, 2019 11:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/3.0.0 #56

Feature/3.0.0 #56

geclos commented May 17, 2019 •

edited

Loading

franciscoj May 21, 2019

geclos May 21, 2019 •

edited

Loading

franciscoj May 21, 2019

masylum May 21, 2019

geclos May 21, 2019 •

edited

Loading

masylum May 22, 2019

masylum May 21, 2019

geclos May 21, 2019 •

edited

Loading

masylum May 22, 2019

geclos May 22, 2019

masylum May 22, 2019

masylum May 21, 2019

geclos May 21, 2019

masylum May 21, 2019

geclos May 21, 2019

masylum May 21, 2019

geclos May 21, 2019 •

edited

Loading

Feature/3.0.0 #56

Feature/3.0.0 #56

Conversation

geclos commented May 17, 2019 • edited Loading

WAT

Choose a reason for hiding this comment

geclos May 21, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geclos May 21, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geclos May 21, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geclos May 21, 2019 • edited Loading

Choose a reason for hiding this comment

geclos commented May 17, 2019 •

edited

Loading

geclos May 21, 2019 •

edited

Loading

geclos May 21, 2019 •

edited

Loading

geclos May 21, 2019 •

edited

Loading

geclos May 21, 2019 •

edited

Loading