0.36.0 changes #126

2019-04-03T17:59:25+02:00

neb-b commented

2019-04-03 17:59:25 +02:00

(Migrated from github.com)

Before you look at the diff count...

Most of these additions are in flow-typed/npm/ and can be ignored. They come from https://github.com/flow-typed/flow-typed and are auto-generated.
Most of the other types inside flow-typed existed before, but have been checked to match the api in 0.36.0
The largest actual additions are in flow-typed/Claim and flow-typed/Lbry

PR Notes

Besides the api changes, I went a little crazy with the current Flow setup but even in it's half-finished state, it's so much nicer than what we have currently.
There are response types for every Lbry method.
- Ideally these would be used automatically but that will be a bit more work.
- For now you can do Lbry.status().then((status: StatusResponse) => ...) and StatusResponse will match the response returned from the SDK. Every method with a usable response should have a type that matches the same pattern, ex: Lbry.claim_list => ClaimListResponse
- To use them in your project, you need to add a line to your .flowconfig like I did here https://github.com/lbryio/lbry-desktop/pull/2407/files#diff-a9d0201e8c5bdc946e54218bb9206174R7

Breaking changes

claim_list_mine is now claim_list
claim_tip is now support_create
- We were already doing this under the hood, but now the method called will actually match the value passed to the sdk
doFetchClaimCountByChannel is removed.
- The claim count is returned and saved when resolve is called with a channel
- https://github.com/lbryio/lbry-redux/pull/126/files#diff-eaffe64d0c2a6b76072de5084117ddf4R59
Now only using rollup instead of webpack
- It was a pain trying to get both to work, rollup is much more preferrable for code splitting.
- The only changed required to use rollup is: https://github.com/lbryio/lbry-desktop/blob/master/webpack.base.config.js#L98

There are a lot of breaking changes related to the data returned, but those should be the only differences re/ calling methods in lbry-redux

### Before you look at the diff count... - Most of these additions are in `flow-typed/npm/` and can be ignored. They come from https://github.com/flow-typed/flow-typed and are auto-generated. - Most of the other types inside `flow-typed` existed before, but have been checked to match the api in 0.36.0 - The largest actual additions are in `flow-typed/Claim` and `flow-typed/Lbry` ### PR Notes - Besides the api changes, I went a little crazy with the current Flow setup but even in it's half-finished state, it's so much nicer than what we have currently. - There are response types for every `Lbry` method. - Ideally these would be used automatically but that will be a bit more work. - For now you can do `Lbry.status().then((status: StatusResponse) => ...)` and `StatusResponse` will match the response returned from the SDK. Every method with a usable response should have a type that matches the same pattern, ex: `Lbry.claim_list` => `ClaimListResponse` - To use them in your project, you need to add a line to your `.flowconfig` like I did here https://github.com/lbryio/lbry-desktop/pull/2407/files#diff-a9d0201e8c5bdc946e54218bb9206174R7 ### Breaking changes - `claim_list_mine` is now `claim_list` - `claim_tip` is now `support_create` - We were already doing this under the hood, but now the method called will actually match the value passed to the sdk - `doFetchClaimCountByChannel` is removed. - The claim count is returned and saved when `resolve` is called with a channel - https://github.com/lbryio/lbry-redux/pull/126/files#diff-eaffe64d0c2a6b76072de5084117ddf4R59 - Now only using rollup instead of webpack - It was a pain trying to get both to work, rollup is much more preferrable for code splitting. - The only changed required to use rollup is: https://github.com/lbryio/lbry-desktop/blob/master/webpack.base.config.js#L98 There are a lot of breaking changes related to the data returned, but those should be the only differences re/ calling methods in `lbry-redux`

skhameneh (Migrated from github.com) reviewed

2019-04-03 17:59:25 +02:00

kauffj (Migrated from github.com) reviewed

2019-04-03 17:59:25 +02:00

neb-b (Migrated from github.com) reviewed

2019-04-15 21:30:16 +02:00

.eslintrc.json

					
				@ -1,19 +1,7 @@

				{

We moved the desktop repo over to the same (kinda) rules that spee.ch is using. Keeping this in line with those.

neb-b (Migrated from github.com) reviewed

2019-04-16 22:39:21 +02:00

src/redux/selectors/claims.js

					
				@ -270,1 +348,4 @@

				  );

				export const makeSelectRecommendedContentForUri = (uri: string) =>

				  createSelector(

Will finish this when I complete the app changes

neb-b commented

2019-04-16 22:46:57 +02:00

(Migrated from github.com)

@tzarebczan If you want, feel free to check this over too. And leave a comment if you think a certain type will change.

tzarebczan (Migrated from github.com) reviewed

2019-04-17 19:07:13 +02:00

flow-typed/Claim.js

					
				@ -0,0 +64,4 @@

				  homepage_url?: string,

				};

				declare type StreamMetadata = GenericMetadata & {

Stream will also include the media type data, and have it's own section for the source information: https://github.com/lbryio/types/blob/master/v2/proto/claim.proto#L20

neb-b (Migrated from github.com) reviewed

2019-04-17 19:33:14 +02:00

flow-typed/Claim.js

					
				@ -0,0 +64,4 @@

				  homepage_url?: string,

				};

				declare type StreamMetadata = GenericMetadata & {

Discussed with Tom. This may change, but keeping it for now.

akinwale (Migrated from github.com) requested changes

2019-04-18 10:19:47 +02:00

akinwale (Migrated from github.com) left a comment

Just a few minor things. Everything else looks good.

dist/flow-typed/Claim.js

It makes sense to have Locations as a single type with all the fields. latitude and longitude is applicable to all kinds of locations.

It makes sense to have `Locations` as a single type with all the fields. `latitude` and `longitude` is applicable to all kinds of locations.

dist/flow-typed/File.js

					
				@ -0,0 +1,35 @@

				// @flow

It looks like the types are being copied to the dist/ folder. Is this intentional?

It looks like the types are being copied to the `dist/` folder. Is this intentional?

src/redux/reducers/claims.js

					
				@ -1,0 +5,4 @@

				// https://github.com/facebook/flow/issues/2221

				// We could move to es6 Sets/Maps, but those are not recommended for redux

				// https://github.com/reduxjs/redux/issues/1499

				// Unsure of the best solution at the momentf

Should be removed before merge, I presume.

src/util/claim.js

					
				@ -3,0 +2,4 @@

				const naughtyTags = ['porn', 'nsfw', 'mature', 'xxx'].reduce(

				  (acc, tag) => ({ ...acc, [tag]: true }),

				  {}

Is this always going to be a fixed list? It is advisable to obtain this from an api call because I imagine this could end up being dynamic as the items listed here are by no means exhaustive.

neb-b (Migrated from github.com) reviewed

2019-04-18 22:09:37 +02:00

src/util/claim.js

					
				@ -3,0 +2,4 @@

				const naughtyTags = ['porn', 'nsfw', 'mature', 'xxx'].reduce(

				  (acc, tag) => ({ ...acc, [tag]: true }),

				  {}

That's a good idea. I'll keep it for now just to get things working then do something similar to how we do the blocked outpoint list.

neb-b (Migrated from github.com) reviewed

2019-04-18 22:11:54 +02:00

dist/flow-typed/Claim.js

Good catch. I'll update.

neb-b (Migrated from github.com) reviewed

2019-04-18 22:13:38 +02:00

dist/flow-typed/File.js

					
				@ -0,0 +1,35 @@

				// @flow

Yep. Then they can be accessed in apps easily. Ideally the Lbry methods would also have the automatic types added, still working on that.

It makes it so you can add a line to the flowconfig, then can just use these as global types.

Yep. Then they can be accessed in apps easily. Ideally the Lbry methods would also have the automatic types added, still working on that. It makes it so you can add a line to the `flowconfig`, then can just use these as global types.

akinwale (Migrated from github.com) approved these changes

2019-04-24 08:41:34 +02:00

akinwale (Migrated from github.com) left a comment

Looks good.

Sign in to join this conversation.

No reviewers

skhameneh

kauffj

akinwale

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

LBRYCommunity/lbry-redux!126

				`@ -1,19 +1,7 @@`
				`{`

				`@ -0,0 +1,35 @@`
				`// @flow`

				`@ -0,0 +1,35 @@`
				`// @flow`

Rows
Columns