moroshko / react-scanner

Extract React components and props usage from code.
MIT License
564 stars 40 forks source link

Add supported node versions to documentation #5

Closed brendanrygus closed 3 years ago

brendanrygus commented 3 years ago

Hi @moroshko, I saw your post on the Design Systems slack, thanks for putting up this useful library. My team and I have talked about this idea a lot recently, but you really pushed ahead with all of the hard work. 🙏

I tested react-scanner out today and ran into a small conflict. It can be easily worked around, but I will raise an issue in case it helps out any other users in the meantime.


Background Trialing react-scanner in our product repo pinned to node v10.16.0.

Issue Internal processor utilities rely on Object.fromEntries, which is not supported in Node v10. This missing helper throws an error Object.fromEntries is not a function when using count-components-and-props. However, package.json for the library specifies the supported "engines" version as 10.12+,

Steps to Reproduce

Toggle config ``` module.exports = { crawlFrom: "./src", includeSubComponents: true, importedFrom: new RegExp("^ds-uikit-uq-web/components/react"), processors: [["count-components-and-props", { outputTo: "./scan.json" }]] }; ```
Toggle error details ``` npx: installed 34 in 3.481s Scanned 5340 files in 26.823932109 seconds (node:93135) UnhandledPromiseRejectionWarning: TypeError: Object.fromEntries is not a function at sortObjectKeysByValue (/Users/me/.npm/_npx/93135/lib/node_modules/react-scanner/src/utils.js:175:17) at forEachComponent (/Users/me/.npm/_npx/93135/lib/node_modules/react-scanner/src/processors/count-components-and-props.js:30:35) at /Users/me/.npm/_npx/93135/lib/node_modules/react-scanner/src/utils.js:150:7 at /Users/me/.npm/_npx/93135/lib/node_modules/react-scanner/src/processors/count-components-and-props.js:8:3 at run (/Users/me/.npm/_npx/93135/lib/node_modules/react-scanner/src/run.js:82:26) at sade.version.describe.option.example.action (/Users/me/.npm/_npx/93135/lib/node_modules/react-scanner/src/index.js:21:7) at Sade.parse (/Users/me/.npm/_npx/93135/lib/node_modules/react-scanner/node_modules/sade/lib/index.js:190:56) at Object. (/Users/me/.npm/_npx/93135/lib/node_modules/react-scanner/src/index.js:37:4) at Module._compile (internal/modules/cjs/loader.js:776:30) at Object.Module._extensions..js (internal/modules/cjs/loader.js:787:10) (node:93135) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1) (node:93135) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code. ```

Workaround Installing Node v12 (nvm use 12) before running react-scanner resolved the issue.

Next Steps Would it be possible to outline the node support strategy in the readme, and update the package.json if necessary? I noticed a couple of automated version bumps were closed (https://github.com/moroshko/react-scanner/pull/3, https://github.com/moroshko/react-scanner/pull/1), so I didn't want to raise a PR making any assumptions.

Thanks!

moroshko commented 3 years ago

Thanks @brendanrygus for raising this! I removed the usage of Object.fromEntries in version 0.4.2. Could you give it a try and confirm that it's working now?

brendanrygus commented 3 years ago

Oh cool, that works too, and should open things to a wider audience (especially those of us working with larger organizations who won't be so quick to upgrade out of v10).

Looks good to me now ✅

nvm use v10.16
Now using node v10.16.0 (npm v6.9.0)

npx react-scanner -c ./scanner.config.js
npx: installed 34 in 2.916s
Scanned 5340 files in 26.021264306 seconds
moroshko commented 3 years ago

Awesome, thanks for confirming!

moroshko commented 3 years ago

@brendanrygus Just wondering, have you found the report useful? Is there anything missing that we should include in the report?

brendanrygus commented 3 years ago

@moroshko Sorry for the delayed response, I've been working on a private repo and stuck in a separate github account🤦‍♂️. The report provided all of the design system usage details I needed at the time - the only gotchas I ran into on the first pass were covered in https://github.com/moroshko/react-scanner/issues/10 https://github.com/moroshko/react-scanner/issues/11

I'll be working with a new design system project soon and look forward to revisiting this as part of our tooling stack