Common components refactor #1
Labels
No labels
area: devops
area: discovery
area: docs
area: livestream
area: proposal
consider soon
dependencies
Epic
good first issue
hacktoberfest
help wanted
icebox
Invalid
level: 1
level: 2
level: 3
level: 4
needs: exploration
needs: grooming
needs: priority
needs: repro
needs: tech design
on hold
priority: blocker
priority: high
priority: low
priority: medium
resilience
Tom's Wishlist
type: bug
type: discussion
type: improvement
type: new feature
type: refactor
type: task
type: testing
unplanned
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: LBRYCommunity/lbry-redux#1
Loading…
Reference in a new issue
No description provided.
Delete branch "common-components-refactor"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
@ -0,0 +1,30 @@
{
It's a dream to have linting and prettier configuration files shared across JS repos, right?
(By dream I mean unrealistic)
@ -1,5 +1,14 @@
module.exports =
/******/ (function(modules) { // webpackBootstrap
(function webpackUniversalModuleDefinition(root, factory) {
This shouldn't be checked in.
@ -17,6 +17,7 @@
"main": "build/index.js",
"scripts": {
"build": "webpack",
"precommit": "lint-staged",
Please remove any unused depdencies.
What's the purpose of this file?
@ -8,116 +9,88 @@ const Lbry = {
pendingPublishTimeout: 20 * 60 * 1000,
};
jsonrpc logic can be moved into this file
Also, all of the properties on
Lbry
can be killed andLbry
can just be the API proxy.@ -4,2 +3,3 @@
const LbryApi = {
const Lbryapi = {
enabled: true,
exchangePromise: null,
I think we should consider naming this something else. If a new user is using this repo to build something, it's confusing to have two objects,
Lbry
, andLbryApi
, and then first one is the LBRY API!Maybe call this
LbryIncApi
orLbryIoApi
? Not in love with either.All of these actions should probably be outside of
lbry-redux
, eh?All of these will need to be removed from other actions.
Should these actions be in
lbry-redux
? I'm inclined to say no.@ -1,5 +1,14 @@
module.exports =
/******/ (function(modules) { // webpackBootstrap
(function webpackUniversalModuleDefinition(root, factory) {
This was generated by webpack. I'll see if I can remove it.
The exports exist to expose names that need to be used from the source file, so that they can be included in the webpack build, which makes them accessible when the lbry-redux package is being used elsewhere. This is similar to how other React packages work. For example, https://github.com/reactjs/react-redux/blob/master/src/index.js
build/index.js is the webpack output.
@ -4,2 +3,3 @@
const LbryApi = {
const Lbryapi = {
enabled: true,
exchangePromise: null,
LbryWebApi or Lbrydotio, maybe?
@ -8,116 +9,88 @@ const Lbry = {
pendingPublishTimeout: 20 * 60 * 1000,
};
We still need the
connect
method which is called on app startup, but I guess this is actually desktop-app specific, so probably needs to be moved to the app repo.It doesn't look like this is using the latest code from
lbry-app
. I changed a few files with the redesign. If it's just a copy/paste I would re-add the new files.It also looks like some of the notification code isn't complete. Maybe we should discuss this tomorrow.
@ -279,0 +200,4 @@
return target[name];
}
return (params = {}) =>
All of the
lbry.publish
code was removed. The weird stuff with pending publishes was removed from this file and is handled inside thepublish
reducer@ -4,2 +3,3 @@
const LbryApi = {
const Lbryapi = {
enabled: true,
exchangePromise: null,
I like
LbryIncApi
.Whatever works though, I agree it might be nicer to have a larger difference between this and
lbry
Did we decide on something like
DO_NOTIFY
?https://github.com/lbryio/lbry-app/blob/master/src/renderer/redux/actions/wallet.js#L12
Yes, we did. I was supposed to delete the
src/redux/actions/app.js
file. It's not used anywhere inlbry-redux
at this point.You can see the notifications related code in
src/redux/actions/notification.js
,src/redux/reducers/notification.js
andsrc/redux/selectors/notification.js
. We'll need to updatelbry-app
to make use of the new action, which will be the next step after the merge is completed.