Closed jsomsanith-tlnd closed 7 years ago
Thanks @jsomsanith the webpack file change will cause the entire tv4 lib to be imported into the library on build. Which negates the need for the other change, however I prefer the package file change only as I don't want to include tv4 as I hope to make it optional in a future update. If you add it back happy to accept the package change, that was my mistake :)
Hi @Anthropic, I'm not sure to understand everything you said.
Which negates the need for the other change
--> what do you call the other change
?
I prefer the package file change only
--> what do you call the package file change
? Do you mean to install tv4 and to add it as external in webpack config ?
Do you agree that the current situation is problematic ? (What I wrote in motivation
part). If so, let's find a solution, because a lib should not be that hard to use.
Yes but at the moment you ask to your users to configure webpack the same way you are doing it (with extra bundle).
With the following change the user will not have to care about what is used to validate the JSON schema. So with this current change: I can use json-schema-form-core without care of what is the validation library inside. So if you do a change one day from tv4 to sth else, this is not a problem for your user.
If you let the current situation, all the users are doing specific webpack config to add tv4. Then you change from tv4 to sth else, all the other users will have to change their webpack config.
This is a dependency so I should not take care of it in my build.
@jmfrancois is right, this means you force everyone to update their webpack config when you change the dependency. That makes the future upgrades possibly harder or more annoying. Obviously, for now tv4 is a dependency, without it this lib doesn't work, it is ok to have it in the bundle.
@jsomsanith yes I agree, tv4 is problematic, I am hoping before I even get to a v1.0.0 release to make it optional, tv4 isn't supported at all any more, it is a real problem as many user issues are raised that are directly caused by it and I can't make changes to. The sooner I can remove it the better, you'll feel the same way very quickly :) I'm just trying to find the best way to do that.
I do take the point that I could include it until that time. However, that means making a breaking change against all the users who include it separately now and then another breaking change when the alternative is ready, I had hoped to only annoy everyone once. How are you including the lib, are you not using webpack?
Anyway what I meant about negating the other change was that if you include tv4 in the build then it should remain as a dev dependency only as it will not need to be included as a lib dependency, so I think you only want the change in the webpack file?
We are not using the lib yet, but we plan too. We encountered the problem at the beginning of a POC, that's why I made the PR. Ok I did not understand that it was in your immediate goal. But please, keep what we said in mind when you'll make the change. I mean that the users should not have to configure anything in their webpack to make the lib work.
Thank you for your time.
@jsomsanith @jmfrancois I think we can discuss such things further here: https://gitter.im/json-schema-form/core
Be nice to communicate more fluidly about what you feel the API may need or things that could make it more useful before I actually start working on it in more detail. I originally expected it to be a pre-packed lib that can be re-packed by any lib wanting to embed it. But I have also recently been thinking about webpack tree shaking and can see the benefit in making it includable as a module that can be stripped of unused features.
But I am happy to accept this PR if you undo the package.json file change so that TV4 gets included for now if you like?
@jsomsanith I've added the contributing file, thanks for letting me know it was missing.
Description
We cannot use json-schema-form-core as an npm lib, du to webpack config that declares
tv4
as externals.Installation :
Use :
Result:
Motivation :
Fixes Related issues
Checklist
@json-schema-form/angular-schema-form-lead