Skip to content

Conversation

@kotarella1110
Copy link
Member

@kotarella1110 kotarella1110 commented Jun 14, 2020

@kotarella1110 kotarella1110 changed the title [WIIP] improve build setting [WIP] improve build setting Jun 14, 2020
@kotarella1110 kotarella1110 changed the title [WIP] improve build setting improve build setting Jun 15, 2020
Comment on lines +29 to +39
"keywords": [
"scheme",
"validation",
"scheme-validation",
"hookform",
"react-hook-form",
"yup",
"joi",
"sperstruct",
"typescript"
],
Copy link
Member Author

Choose a reason for hiding this comment

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

@bluebill1049 Is this keyword OK?

Copy link
Member

Choose a reason for hiding this comment

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

awesome! thanks

rollup.config.js Outdated
{
input: 'src/index.ts',
output: {
name: 'HookFormResolvers',
Copy link
Member Author

Choose a reason for hiding this comment

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

@bluebill1049 Is this umd name OK?
HookFormResolvers vs ReactHookFormResolvers vs Resolvers
please see umd example.

Copy link
Member

Choose a reason for hiding this comment

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

ReactHookFormResolvers 👍 what do you think? looks better at umd example.

Copy link
Member Author

Choose a reason for hiding this comment

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

we may use HookForm as the umd name of react-hook-form.
Which is better?

  • HookForm
  • HookFormResolvers
  • HookFormErrorMessage
  • HookFormDevtools

vs

  • ReactHookForm
  • ReactHookFormResolvers
  • ReactHookFormErrorMessage
  • ReactHookFormDevtools

Copy link
Member

Choose a reason for hiding this comment

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

actually maybe second one, what if one day we have angularHookForm? lol

Copy link
Member Author

Choose a reason for hiding this comment

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

Hahaha. if it has the potential to support Angular and Vue, I think ReactHookForm is better.
However, in that case we think it is necessary to reconsider the scope name (@hookform).
ex: @hookform to @react-hook-form

Copy link
Member

Choose a reason for hiding this comment

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

@hookform/vue is fine as well?

if we can change to @hookform/react later on.

Copy link
Member Author

Choose a reason for hiding this comment

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

awesome! I will change umd name from HookForm to ReactHookForm.

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

@kotarella1110
Copy link
Member Author

@bluebill1049 I will fix @hookform/devtools as well.

@@ -1,39 +0,0 @@
example/
Copy link
Member

Choose a reason for hiding this comment

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

why remove this?

Copy link
Member Author

@kotarella1110 kotarella1110 Jun 16, 2020

Choose a reason for hiding this comment

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

I fixed to use files, we can remove the .npmignore.

https://docs.npmjs.com/files/package.json#files

resolvers/package.json

Lines 13 to 15 in 57795af

"files": [
"dist"
],

Copy link
Member

Choose a reason for hiding this comment

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

can you do the same for react-hook-form?

Copy link
Member Author

Choose a reason for hiding this comment

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

yea! I will fix 👍

@@ -1,4 +0,0 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Member Author

Choose a reason for hiding this comment

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

I moved prettier setting to .eslintrc.js.
I think it's more useful to be able to manage with one file.
What do you think?

resolvers/.eslintrc.js

Lines 15 to 21 in 57795af

'prettier/prettier': [
'error',
{
singleQuote: true,
trailingComma: 'all',
},
],

Copy link
Member

Choose a reason for hiding this comment

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

personally, I prefer to keep them separate. They are two different technology, then others may ask why not keep eslint inside prettier.

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

README.md Outdated

[![Tweet](https://img.shields.io/twitter/url/http/shields.io.svg?style=social)](https://twitter.com/intent/tweet?text=React+hooks+for+form+validation+without+the+hassle&url=https://github.com/bluebill1049/@hookform/resolvers) [![Join the community on Spectrum](https://withspectrum.github.io/badge/badge.svg)](https://spectrum.chat/react-hook-form)
[![Tweet](https://img.shields.io/twitter/url/http/shields.io.svg?style=social)](https://twitter.com/intent/tweet?text=React+hooks+for+form+validation+without+the+hassle&url=https://github.com/react-hook-form/resolvers)
[![Join the community on Spectrum](https://withspectrum.github.io/badge/badge.svg)](https://spectrum.chat/react-hook-form)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[![Join the community on Spectrum](https://withspectrum.github.io/badge/badge.svg)](https://spectrum.chat/react-hook-form)

I think going forward we are leaner toward github dicussion.

Copy link
Member Author

Choose a reason for hiding this comment

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

done 👍

README.md Outdated
[![npm](https://img.shields.io/bundlephobia/minzip/@hookform/resolvers?style=for-the-badge)](https://bundlephobia.com/result?p=@hookform/resolvers)

[![Tweet](https://img.shields.io/twitter/url/http/shields.io.svg?style=social)](https://twitter.com/intent/tweet?text=React+hooks+for+form+validation+without+the+hassle&url=https://github.com/bluebill1049/@hookform/resolvers) [![Join the community on Spectrum](https://withspectrum.github.io/badge/badge.svg)](https://spectrum.chat/react-hook-form)
[![Tweet](https://img.shields.io/twitter/url/http/shields.io.svg?style=social)](https://twitter.com/intent/tweet?text=React+hooks+for+form+validation+without+the+hassle&url=https://github.com/react-hook-form/resolvers)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[![Tweet](https://img.shields.io/twitter/url/http/shields.io.svg?style=social)](https://twitter.com/intent/tweet?text=React+hooks+for+form+validation+without+the+hassle&url=https://github.com/react-hook-form/resolvers)
[![Tweet](https://img.shields.io/twitter/url/http/shields.io.svg?style=social)](https://twitter.com/intent/tweet?text=React+hooks+for+form+validation+without+the+hassle&url=https://github.com/react-hook-form/resolvers)

don't think anyone use it. let's remove it as well keep it clean and minimum.

Copy link
Member Author

Choose a reason for hiding this comment

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

done 👍

Copy link
Member

@bluebill1049 bluebill1049 left a comment

Choose a reason for hiding this comment

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

Awesome work and clean up! thank you.

Copy link
Member

@bluebill1049 bluebill1049 left a comment

Choose a reason for hiding this comment

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

LGTM!

@kotarella1110
Copy link
Member Author

kotarella1110 commented Jun 16, 2020

Would you also like to fix the output name of react-hook-form? Or do you want to leave it as is?

  • dist/react-hook-form.js to dist/index.js
  • dist/react-hook-form.es.js to dist/index.esm.js
  • dist/react-hook-form.umd.js to dist/index.umd.js
  • dist/react-hook-form.ie11.js to dist/index.ie11.js

https://github.com/react-hook-form/react-hook-form/blob/0fee0da0044e1cd3afa887fa046dd84e92e4ed44/package.json#L5-L6

@bluebill1049
Copy link
Member

Would you also like to fix the output name of react-hook-form? Or do you want to leave it as is?

  • dist/react-hook-form.js to dist/index.js
  • dist/react-hook-form.es.js to dist/index.esm.js
  • dist/react-hook-form.umd.js to dist/index.umd.js
  • dist/react-hook-form.ie11.js to dist/index.ie11.js

https://github.com/react-hook-form/react-hook-form/blob/0fee0da0044e1cd3afa887fa046dd84e92e4ed44/package.json#L5-L6

sounds good to me. as long as we can do a quick test, you can still easily find the source code under chrome developer tab.

@kotarella1110
Copy link
Member Author

I will fix it 👍

@kotarella1110
Copy link
Member Author

This PR includes a version release, which is better, squash merge or merge commit?

@bluebill1049
Copy link
Member

This PR includes a version release, which is better, squash merge or merge commit?

I normally run npm version at master and push the tag into master, but whatever works.

@kotarella1110
Copy link
Member Author

@bluebill1049 "Create a merge commit" OK?

スクリーンショット 2020-06-16 11 32 56

@bluebill1049
Copy link
Member

squash merge, please :)

@kotarella1110 kotarella1110 merged commit 28948ec into master Jun 16, 2020
@kotarella1110 kotarella1110 deleted the improve-build-setting branch June 16, 2020 03:03
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

Successfully merging this pull request may close these issues.

3 participants