Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

fix: remove react from distributed bundle #104

Merged
merged 3 commits into from
May 26, 2021
Merged

fix: remove react from distributed bundle #104

merged 3 commits into from
May 26, 2021

Conversation

lwhiteley
Copy link
Collaborator

relates to #96

@lwhiteley lwhiteley requested a review from iambumblehead May 26, 2021 05:07
@github-actions
Copy link

github-actions bot commented May 26, 2021

🎊 PR Preview 1b83f08 has been successfully built and deployed to https://iambumblehead-react-dropdown-now-preview-pr-104.surge.sh

🕐 Build time: 120.126s

🤖 By surge-preview

@lwhiteley
Copy link
Collaborator Author

lwhiteley commented May 26, 2021

@dalmo3 @iambumblehead

please pull branch locally and let me know if it. works

  • re-install dependencies (maybe delete node_modules folder)

if using npm 7 (npm i --legacy-peer-deps )

@dalmo3
Copy link

dalmo3 commented May 26, 2021

Still no luck. Same error. Invalid hook call in Dropdown.tsx. Tried with npm 7 like you suggested and yarn v1.

PS. Node 15.3

@lwhiteley
Copy link
Collaborator Author

lwhiteley commented May 26, 2021

hmm doesnt add up .. i used same env and worked.
unless im not testing properly to reproduce the issue

Steps:
git checkout fix-lockfile

Test 1:

  • delete node_modules
  • yarn
  • yarn start

Test 2:

  • delete node_modules
  • npm i --legacy-peer-deps
  • npm start

Both worked

Screenshot 2021-05-26 at 7 59 28 AM
Screenshot 2021-05-26 at 8 05 06 AM

@iambumblehead
Copy link
Owner

iambumblehead commented May 26, 2021

I just discovered this PR... I haven't tried running @dalmo3's sample app on the linux machine but I'll try it and see how it goes. There's a similar error that occurs when running storybook storybookjs/storybook#10662 (comment) but the error only occurs on the mac machine --storybook runs fine on the linux machine

edit later on: same Invalid hook call error occurs on both machines, using @dalmo3's sample app

mac linux
node v16.0.0 node v16.2.0
npm 7.10.0 npm 7.14.0
Error: Invalid hook call Error: Invalid hook call

I also updated the sample app to use the exact same version of react and react-dom and so that would rule out mismatched versions of react-dom as causing the error, but possibly multiple versions of react are causing the error

bumble@duck react-dropdown-now-app水$ npm ls react-dom
[email protected] /home/bumble/software/react-dropdown-now-app
└── [email protected]

bumble@duck react-dropdown-now-app水$ npm ls react
[email protected] /home/bumble/software/react-dropdown-now-app
├─┬ [email protected]
│ └── [email protected] deduped
└── [email protected]

many other people are recently affected by the same sort of problems facebook/react#13991

also related, this recent PR (by me) removed peerDependencies and maybe it needs to be re-added. I re-added it here locally but it did not resolve the error,

{
  "peerDependencies": {
    "react": ">=16.8.3",    
    "react-dom": ">=16.8.3"
  }
}

@lwhiteley
Copy link
Collaborator Author

still no issues for me

mac
node v16.2.0
npm 7.14.0
all fine

@lwhiteley
Copy link
Collaborator Author

many other people are recently affected by the same sort of problems facebook/react#13991

yup but this issue is from 2018.
Im not certain if im reproducing the issue as you guys are doing it.

can you give detailed instructions and what you are using to test the issue

@iambumblehead
Copy link
Owner

react-dropdown-now-app.zip

This is a version of @dalmo3's app. To use it, unzip it and npm install && npm start.

To test this branch there rm node_modules/react-dropdown-now then clone this branch to the same place, npm install && npm build.

@iambumblehead
Copy link
Owner

@lwhiteley the bottom, recent half of the discussion in that thread is from the last 30 days or so facebook/react#13991 (comment)

@lwhiteley
Copy link
Collaborator Author

lwhiteley commented May 26, 2021

@iambumblehead

in the bundle you have

"react-dropdown-now": "6.0.0",. shouldnt this be "react-dropdown-now": "../react-dropdown-now", ?

also FYI npm 7 is very strict

Found: [email protected]
npm ERR! node_modules/typescript
npm ERR!   dev typescript@"4.1.3" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peerOptional typescript@"^3.2.1" from [email protected]
npm ERR! node_modules/react-scripts
npm ERR!   react-scripts@"4.0.0" from the root project
npm ERR! 

ill have to use legacy-peer-deps

@lwhiteley
Copy link
Collaborator Author

ok now i reproduce. will continue from here later

@lwhiteley
Copy link
Collaborator Author

it seems to not be installing changes from the branch somwhow and still old version

i removed the useMemo but it still shows that code
Screenshot 2021-05-26 at 10 18 31 AM

@iambumblehead
Copy link
Owner

@lwhiteley yes you are right and my explanation above was maybe not very good. I downloaded the dependencies for the app, including react-dropdown-now 6.0.0, then I deleted react-dropdown-now and cloned this branch into the same location so that running npm start from the parent app finds the cloned instance of react-dropdown-now rather than the (deleted) version 6.0.0

@lwhiteley
Copy link
Collaborator Author

hmm i didnt rebuild my local dist folder..will test again soon

@iambumblehead
Copy link
Owner

Screenshot from 2021-05-26 01-40-51

the @reach/router react dependency (from storybook) makes it impossible to control the react dependencies. We used the work-around described here reach/router#432 (comment) in order to install storybook with the react17

@lwhiteley
Copy link
Collaborator Author

yup but when you install the component as a dependency, storybook isnt installed so something else is the issue

@lwhiteley
Copy link
Collaborator Author

so i confirmed that our bundle had a react version inside and this removes it..however your setup with npm link still has issue so ill fix this up and we do a patch release to test correctly

@lwhiteley
Copy link
Collaborator Author

lwhiteley commented May 26, 2021

i also confirmed the test app is working actually

instead of using npm link

  • i just installed v6.0.0
  • npm run build on this local branch
  • copied the dist folder into the node_module of the test app

Screenshot 2021-05-26 at 12 49 32 PM

@lwhiteley lwhiteley changed the title fix: update lock file fix: remove react from distributed bundle May 26, 2021
@lwhiteley
Copy link
Collaborator Author

k @iambumblehead .. you can do a patch release

Thanks

@lwhiteley lwhiteley merged commit 84557b7 into master May 26, 2021
@lwhiteley lwhiteley deleted the fix-lockfile branch May 26, 2021 16:40
@iambumblehead
Copy link
Owner

@lwhiteley your latest branch works here after removing react-dropdown-now's node modules directory

bumble@duck react-dropdown-now-app水$ cd node_modules/react-dropdown-now/
bumble@duck react-dropdown-now水$ mv node_modules node_modulesOLD

great! :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

onChange and onSelect shouldn't fire if value is changed programmatically
3 participants