Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid hook call. #6

Open
cawfree opened this issue Oct 5, 2021 · 2 comments
Open

Invalid hook call. #6

cawfree opened this issue Oct 5, 2021 · 2 comments

Comments

@cawfree
Copy link

cawfree commented Oct 5, 2021

When nesting my application within the <KeyboardShift />, it fails to render.

There's a couple of reasons why this could be:

  1. The entire dependency stack for this library (including a duplicate version of React) is included. Many of these could be moved to devDependencies/peerDependencies, with an explicit statement that the library depends upon @react-navigation/elements.
  2. This line which depends on conditional hook execution.

I was able to resolve the issue locally by installing @react-navigation/elements to my root project and deleting the local node_modules directory installed at node_modules/\@fullstackcraft/.

The package.json could look something like:

{
  "name": "@fullstackcraft/react-native-keyboard-shift",
  "version": "1.1.4",
  "description": "A 2020s compatible keyboard avoiding view for Android and iOS that just works.",
  "main": "lib/index.js",
  "types": "lib/index.d.ts",
  "scripts": {
    "test": "jest --config jestconfig.json",
    "build": "tsc",
    "format": "prettier --write \"src/**/*.ts\" \"src/**/*.tsx\"",
    "lint": "tslint -p tsconfig.json",
    "prepare": "npm run build",
    "prepublishOnly": "npm test && npm run lint",
    "preversion": "npm run lint",
    "version": "npm run format && git add -A src",
    "postversion": "git push && git push --tags"
  },
  "dependencies": {
    "@react-native-community/hooks": "^2.6.0",
-   "@react-navigation/elements": "^1.1.2",
+   "@react-navigation/elements": "^1.1.2"
-    "react": ">= 17.0.2",
-    "react-native": ">=0.64.0"
  },
  "repository": {
    "type": "git",
    "url": "git+https://github.com/FullStackCraft/react-native-keyboard-shift.git"
  },
  "keywords": [
    "react",
    "native",
    "keyboard",
    "keyboardshift",
    "keyboard",
    "shift",
    "android",
    "ios"
  ],
  "author": "Chris Frewin",
  "license": "MIT",
  "bugs": {
    "url": "https://github.com/FullStackCraft/react-native-keyboard-shift/issues"
  },
  "homepage": "https://github.com/FullStackCraft/react-native-keyboard-shift#readme",
  "devDependencies": {
    "@types/jest": "^27.0.2",
    "@types/node": "^16.10.2",
    "@types/react": "^17.0.26",
    "@types/react-native": "^0.65.3",
    "jest": "^27.2.4",
    "prettier": "^2.4.1",
+   "react": ">= 17.0.2",
+   "react-native": ">=0.64.0",
    "ts-jest": "^27.0.5",
    "tslint": "^6.1.3",
    "tslint-config-prettier": "^1.18.0"
  },
  "files": [
    "lib/**/*"
  ]
}
@TheRogue76
Copy link
Collaborator

Hey @cawfree. Sorry for the delayed response. I haven't been too well.

About the line you singled out, while i was able to get it to work on the example project without @react-navigation/elements, i agree that having an optional hook could be problematic. I was trying to get the project to work with and without react-navigation present, as there are users who use react-native-navigation.

we could use the available props hasHeader and headerOffset and just ask the user to handle the header situation. This would allow us to get rid of @react-navigation/elements dependency as a package dependency but it would force the user to either write a wrapper for our component to give it the headerOffset or constantly force them to call these props. @princefishthrower @matinzd i want to know your opinion on this as well.

About the changes to the package.json file, i'll try your suggestions tonight and report back. i was running into issues with devDependencies when i moved them but it could have been something else.

@princefishthrower
Copy link
Contributor

@cawfree @TheRogue76 - for the dependency issue, I agree that react and react-native should be removed from dependencies, and rather be peerDependencies. This is what other react-native packages do, for example react-native-async-storage:

https://github.com/react-native-async-storage/async-storage/blob/master/package.json

or react-native-svg:

https://github.com/react-native-svg/react-native-svg/blob/develop/package.json

and many other examples that can be found in react-native packages. I believe then that @react-navigation/elements is the final point - it could remain strictly a dependency or we can do as @TheRogue76 suggests - but then down the road we will likely run into dealing with issues raised in this repo for custom solutions that users build for the headerOffset, when in reality those problems won't be the fault of this package. Alternatively, we could advertise this component strictly as a solution for keyboard shifting views for apps that use react-navigation

TheRogue76 added a commit that referenced this issue Oct 9, 2021
…using elements until we come to a decision on how to handle offsetting. related to #6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants