fix a bug and add a feature #4

kessler · 2019-09-18T14:55:36Z

sometimes the reducer gets called twice because it's an anonymous function, this is fixed by using useCallback hook on the reducer with dependency on initialState:

 const [state, _dispatch] = useReducer((state, action) => reduceState(reducers, state, action), initialState);

=>

const memoizedReducer = useCallback((state, action) => reduceState(reducers, state, action), [initialState])
const [state, _dispatch] = useReducer(memoizedReducer, initialState);

Allow the user of the package to provide their own ConnectState implementation

Do you want me to rebase this or something?
Do you need me to provide a code that recreates the bug?

- publish to personal npm scope - remove webpack build and dev deps - use index.js in package.json as main - relocate src/* to lib/* - expose `ConnectState` component so it can be subclassed - update docs

by applying a useCallback memoization

Jahans3 · 2019-11-21T20:54:08Z

Hey, thanks for this PR. Will take a look over it now - would also be great if you could reproduce the bug too. In the meantime could we keep changes to what is necessary only please

Jahans3

Thanks again for the PR 👍 left some reviews for you to take a look at

Jahans3 · 2019-11-21T20:57:20Z

.npmignore

@@ -1,7 +1,6 @@
 yarn.lock
 yarn-error.log
 uss-logo.png
-src/


Why rename this folder?

It's more common to use the lib folder name than src, if it bothers you I can rename that again.

Yep - necessary changes only please, not personal preference.

Jahans3 · 2019-11-21T21:04:40Z

README.md

@@ -1,3 +1,8 @@
+# this is a fork of [Jahans3/use-simple-state](https://github.com/Jahans3/use-simple-state)


Could we remove before merging please

Jahans3 · 2019-11-21T21:05:24Z

lib/components.js

-class ConnectState extends Component {
-  state = {};
+export class ConnectState extends Component {
+  constructor(props) {


Can we remove this constructor please

I'm calling super() in the ctor, so I don't think it can be removed (or will it be called automagically somehow?)

It's just not necessary in this instance. What you've written is functionally identical to state = {}

Jahans3 · 2019-11-21T21:07:55Z

package.json

-  "peerDependencies": {
-    "react": "^16.7.0"
-  },
-  "devDependencies": {


Can we get the dev deps back please

Can you elaborate why it's needed? I was trying to reduce dependencies and files in the code

Because they're necessary to develop the library. Dev dependencies have zero bearing on the final artefact. I would strongly recommend reading up on dev, peer and normal dependencies

Jahans3 · 2019-11-21T21:08:55Z

webpack.config.js

@@ -1,37 +0,0 @@
-const path = require('path');


Can we get this back please

Same as above, can you elaborate why it's needed? I was trying to reduce dependencies and files in the code

A webpack config is required to use webpack and build the project...

Jahans3 · 2019-11-21T21:09:51Z

yarn.lock

@@ -1,2747 +0,0 @@
-# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.


Can you regenerate this lock file please

lock files are not recommended for libraries, see my comment below about package-lock.json and maybe it applies to yarn as well, I don't actually know much about yarn tbh

The lockfile is still useful for anyone developing on this project.

Jahans3 · 2019-11-21T21:10:40Z

.npmrc

@@ -0,0 +1 @@
+package-lock=false


Could we delete this file please - using yarn on this project and prefer to have lock files in place

npm will ignore lock files in dependencies:

One key detail about package-lock.json is that it cannot be published, and it will be ignored if found in any place other than the toplevel package.

from: https://docs.npmjs.com/files/package-lock.json

This simply disables the lockfile entirely - meaning npm will not generate one. That is not behaviour we want nor do we want to use npm either - this project uses yarn.

Jahans3 · 2019-11-25T16:06:18Z

Also need to see a repro of the bug

kessler · 2019-12-02T17:00:13Z

I'm going to close this as I don't have the time right now to perform all the changes / create a test case for the bug.
Will reopen when I have a bit more time on my hands...

Jahans3 · 2019-12-19T16:28:18Z

@kessler No problem. Will try and recreate the bug in the meantime.

Thanks again for contributing!

kessler added 10 commits August 25, 2019 06:37

allow for custom ConnectState to be used

2e1b2d1

- publish to personal npm scope - remove webpack build and dev deps - use index.js in package.json as main - relocate src/* to lib/* - expose `ConnectState` component so it can be subclassed - update docs

0.1.0

a7433bf

chore: update readme to match styles

c0e578b

don't use package-lock.json

2a07a84

remove class properties

c25bd39

0.1.1

5a4b89f

bug: fix ConnectState ctor

e706960

0.1.2

413543f

bug: fix reducer being called twice

e9b5733

by applying a useCallback memoization

0.1.3

ab3d875

Jahans3 reviewed Nov 21, 2019

View reviewed changes

kessler closed this Dec 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix a bug and add a feature #4

fix a bug and add a feature #4

kessler commented Sep 18, 2019 •

edited

Loading

Jahans3 commented Nov 21, 2019 •

edited

Loading

Jahans3 left a comment

Jahans3 Nov 21, 2019

kessler Nov 21, 2019

Jahans3 Nov 25, 2019

Jahans3 Nov 21, 2019

kessler Nov 21, 2019

Jahans3 Nov 21, 2019

kessler Nov 21, 2019

Jahans3 Nov 25, 2019 •

edited

Loading

Jahans3 Nov 21, 2019

kessler Nov 21, 2019

Jahans3 Nov 25, 2019

Jahans3 Nov 21, 2019

kessler Nov 21, 2019

Jahans3 Nov 25, 2019

Jahans3 Nov 21, 2019

kessler Nov 21, 2019

Jahans3 Nov 25, 2019

Jahans3 Nov 21, 2019

kessler Nov 21, 2019

Jahans3 Nov 25, 2019

Jahans3 commented Nov 25, 2019

kessler commented Dec 2, 2019

Jahans3 commented Dec 19, 2019

		@@ -1,3 +1,8 @@
		# this is a fork of [Jahans3/use-simple-state](https://github.com/Jahans3/use-simple-state)

		@@ -1,2747 +0,0 @@
		# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.

		@@ -0,0 +1 @@
		package-lock=false

fix a bug and add a feature #4

fix a bug and add a feature #4

Conversation

kessler commented Sep 18, 2019 • edited Loading

Jahans3 commented Nov 21, 2019 • edited Loading

Jahans3 left a 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

Jahans3 Nov 25, 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

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

Jahans3 commented Nov 25, 2019

kessler commented Dec 2, 2019

Jahans3 commented Dec 19, 2019

kessler commented Sep 18, 2019 •

edited

Loading

Jahans3 commented Nov 21, 2019 •

edited

Loading

Jahans3 Nov 25, 2019 •

edited

Loading