Skip to content

Improve field loading method #505

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

Merged

Conversation

lionel-bijaoui
Copy link
Member

@lionel-bijaoui lionel-bijaoui commented Oct 4, 2018

  • Please check if the PR fulfills these requirements
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Feature
  • What is the current behavior? (You can also link to an open issue here)

#244

  • What is the new behavior (if this is a feature change)?
  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

There is a new way to deal with fields.
VFG used to register core and/or optionnal fields in Vue (depending on the version you decided to use).
That mean that internally, VFG was using Vue.component() to register all fields.
So if you wanted to use only one field or two, your final app had at best, all "core" fields bundled inside.

Now, no fields are added to Vue by default.
You need to pass them as an option.

// Before
import VueFormGenerator from "vue-form-generator";
Vue.use(VueFormGenerator); // All fields are passed through Vue.component()

// Now
import VueFormGenerator from "vue-form-generator";
import {
  fieldCheckbox,
  fieldInput,
  fieldSubmit
} from "vue-form-generator/utils/fieldsLoader.js";
Vue.use(VueFormGenerator, {
  fields: [fieldCheckbox, fieldInput, fieldSubmit] // Only those fields will pass through Vue.component()
});

It still work with custom fields. Meaning now, you can use VFG with your custom fields only.
(If you use a recent version of Webpack or a tool that does tree-shaking, you should have a better bundle size.
Your custom field will need to have the "name" property set to "field-***" (e.g. "field-awesome")

import VueFormGenerator from "vue-form-generator";
import fieldAwesome from "./fieldAwesome.vue";
Vue.use(VueFormGenerator, {
  fields: [fieldCheckbox, fieldInput, fieldSubmit] // Only those fields will pass through Vue.component()
});

If you want everything like before (full bundle), you need to do that

import VueFormGenerator from "vue-form-generator";
import {
  fieldCheckbox,
  fieldChecklist,
  fieldInput,
  fieldLabel,
  fieldRadios,
  fieldSelect,
  fieldSubmit,
  fieldTextArea,
  fieldUpload,
  fieldCleave,
  fieldDateTimePicker,
  fieldGoogleAddress,
  fieldImage,
  fieldMasked,
  fieldNoUiSlider,
  fieldPikaday,
  fieldRangeSlider,
  fieldSelectEx,
  fieldSpectrum,
  fieldStaticMap,
  fieldSwitch,
  fieldVueMultiSelect
} from "vue-form-generator/utils/fieldsLoader.js";

Vue.use(VueFormGenerator, {
  fields: [
    fieldCheckbox,
    fieldChecklist,
    fieldInput,
    fieldLabel,
    fieldRadios,
    fieldSelect,
    fieldSubmit,
    fieldTextArea,
    fieldUpload,
    fieldCleave,
    fieldDateTimePicker,
    fieldGoogleAddress,
    fieldImage,
    fieldMasked,
    fieldNoUiSlider,
    fieldPikaday,
    fieldRangeSlider,
    fieldSelectEx,
    fieldSpectrum,
    fieldStaticMap,
    fieldSwitch,
    fieldVueMultiSelect
  ]
});
  • Other information:

Be aware that this is the first step toward removing fields from the library and into their own repository.
Soon, VFG will bundle no field at all, and you will need to add them as dependency.

Also, VFG is completely written in ES6 now (import/export), no more CommonJS syntax (require/module.exports). The mix of both caused a bug in Webpack.

@zoul0813
Copy link
Member

zoul0813 commented Oct 4, 2018

is there a way to implement a quick and easy "import all" option? This way we don't have to explicitly list the fields we would have access too. Having to identify each field might be a bit problematic in apps that use dynamic forms ... where we don't know which fields a form will use ahead of time, and therefore have to have all fields loaded and ready to go?

import { CoreFields } from '@vue-form-generator/fields';

Where CoreFields or whatever it would be called, would export an array that can just be passed into Vue.use to register all "core" fields?

@lionel-bijaoui
Copy link
Member Author

lionel-bijaoui commented Oct 4, 2018

Remember that we want to remove all fields from VFG. Soon that will not be possible anyway. You will have to add the fields in your dependencies.
That's a trade-off ? 🤷‍♂️

@zoul0813
Copy link
Member

zoul0813 commented Oct 4, 2018

I thought we were keeping the standard/core fields ... such as input, select, checkbox, etc ... as these are the backbone of all other fields and common across just about any form?

Not sure I agree with removing "all" fields from VFG Core.

@lionel-bijaoui
Copy link
Member Author

lionel-bijaoui commented Oct 4, 2018

Having to identify each field might be a bit problematic in apps that use dynamic forms

They can register all the fields if they want. In the end, this only use Vue.component().

I thought we were keeping the standard/core fields ... such as input, select, checkbox, etc ... as these are the backbone of all other fields and common across just about any form?

Not sure I agree with removing "all" fields from VFG Core.

Don't worry, this is open for debate, but when we talked about this earlier, I understood you wanted to remove them all.
We could keep "core" field in VFG, move the rest into their own repo. No problem! 😄
I will try something tomorrow.

After that, I will do a PR that remove Pug and use more strict EsLint rules.
Pug is stopping eslint-plugin-vue from doing it's job properly and I think this is an obstacle to new contributor. eslint-plugin-vue in vue cli 3 can fix almost everything in a simple npm run lint.

EDIT: BTW, we could also move every field into their own repo and make the "core" field a dependency of VFG. Moving them was also to deal with issues in a more compartmentalized way.

@zoul0813
Copy link
Member

zoul0813 commented Oct 4, 2018

Moving all the core fields into a single package, and flagging it as a dep of the main package is an ideal solution. That would allow us to keep the main package smaller, and focused on the task of "generating the form" and the core-fields package could be focused on presenting the common "everyday" fields. AbstractField and other common-reusable bits should still be part of the main form-generator package though, so third-party fields can extend from main... say for example, in forms where all elements are "custom" (for specific CSS frameworks, etc).

This would allow us to have a "core-bootstrap" and "core-blueprint" package for example, where the common fields are tailored toward either Bootstrap or Blueprint CSS frameworks, etc.

@lionel-bijaoui
Copy link
Member Author

I really want to do a repo by field, and avoid big pack like this.
If I update field-input, I don't want to have to update a big pack, just field-input.
If there is an issue, I want it focused on this single field.
If someone want to contribute to only this one field they use, they can.
We could use the embedded Wiki for documentation and make it super easy.

In this I see also a need to group abstract field, slugify, dateFieldHelper in a npm module (vfg-utils ?), so it could be used by fields without the need of all VFG.
Same thing with validators. Maybe use the same system (options.validator) to include other validation system ?

The "dev" folder need to be made into a submodule, so we can have a repo just to launch example (for #32 )

@lionel-bijaoui lionel-bijaoui merged commit f7e9ea1 into vue-generators:v3 Oct 5, 2018
@lionel-bijaoui lionel-bijaoui deleted the improve-field-loading branch October 5, 2018 12:44
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.

2 participants