Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

✨ Extract proptypes of components with HOCs #49

Merged
merged 12 commits into from
May 2, 2017
Merged

Conversation

bpetetot
Copy link
Contributor

#48

@bpetetot bpetetot requested a review from fabienjuif April 28, 2017 09:30
package.json Outdated
@@ -39,7 +39,8 @@
"sass-loader": "^6.0.3",
"style-loader": "^0.16.1",
"webpack": "^2.3.3",
"webpack-dev-middleware": "^1.10.1"
"webpack-dev-middleware": "^1.10.1",
"whatwg-fetch": "^2.0.3"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

babel-polyfill

throw importType
}
recast.visit(ast, {
visitFunctionDeclaration: false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try the default definition, if this is false you could delete those lines

visitDoWhileStatement: false,
visitForStatement: false,
visitForInStatement: false,
visitExportDeclaration: false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about this one ?

}
return newpath
} catch (e) {
// file path not found
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😨

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore only 1 error, you have to throw the others

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand your last comment

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

} else {
const resolved = utils.resolveToValue(resolveHOC(definition))
if (resolved.node.type === 'ImportDeclaration') {
// Manage imported components in HOC from different file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you pass by an exception instead of recalling exportDeclaration(newPath)

try {
const stats = await fs.statAsync(newpath)
if (stats.isDirectory()) {
return resolvePath(p.resolve(path, 'index.js'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return resolvePath(p.resolve(path, 'index'))


const resolvePath = async (path) => {
const extensions = ['.js', '.jsx', '']
for (let i = 0; i < extensions.length; i += 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for of

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forbidden by the linter :
[eslint] iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations. (no-restricted-syntax)

here the airbnb explanation :
airbnb/javascript#1271

const docgen = require('react-docgen')

const utils = require('react-docgen/dist/utils')
const resolveHOC = require('react-docgen/dist/utils/resolveHOC.js').default
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need that .js

const utils = require('react-docgen/dist/utils')
const resolveHOC = require('react-docgen/dist/utils/resolveHOC.js').default

const isComponentDefinition = (path) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the source (url) in a commentary in this file

Copy link
Contributor

@fabienjuif fabienjuif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above commentaries

@bpetetot bpetetot removed the not-ready label May 1, 2017
@bpetetot
Copy link
Contributor Author

bpetetot commented May 1, 2017

@fabienjuif ready to review

@bpetetot bpetetot merged commit efac37e into master May 2, 2017
@bpetetot bpetetot deleted the resolve-hoc branch May 2, 2017 15:51
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.

2 participants