Skip to content
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

Validation (internal code): evaluate a single entrypoint API #362

Open
rxaviers opened this issue Dec 9, 2014 · 4 comments
Open

Validation (internal code): evaluate a single entrypoint API #362

rxaviers opened this issue Dec 9, 2014 · 4 comments

Comments

@rxaviers
Copy link
Member

rxaviers commented Dec 9, 2014

Jörn raised this concern in #351 (comment). Evaluate the pros and cons of this change.

Currently, no final dist/ have unused validations. So, it's important to determine how this change would affect that and how would it affect its final sizes.


src/currency.js @ header

@jzaefferer

Have you considered making this a single validate module that exports an object with multiple functions? Would help reduce the number of dependencies you need to require in many places, including these modules itself, since most share the same dependencies."

@rxaviers

Different methods require different validations. Therefore, the dependency of the validate functions across Globalize (core), date module, number module, plural module, etc are different.

How bundling all validations on a single export affect the final dist files? What the gain is?"

@jzaefferer

There are 103 effective LOC in common/validate, counting only the returned functions, excluding the define wrappers and blank lines. Grouping those 14 files into a single one with about 120 LOC would simplify dependencies in many places and improve the overall understandibility (I may have made that word up, I hope you catch my meaning).

It would also give the potential for other refactorings, as refactorings in general do, for example I suspect that some of the other modules in common are only used in the various validation modules.

@jzaefferer
Copy link
Contributor

From the original PR:

There are 103 effective LOC in common/validate, counting only the returned functions, excluding the define wrappers and blank lines. Grouping those 14 files into a single one with about 120 LOC would simplify dependencies in many places and improve the overall understandibility (I may have made that word up, I hope you catch my meaning).

It would also give the potential for other refactorings, as refactorings in general do, for example I suspect that some of the other modules in common are only used in the various validation modules.

I estimate that we'd have to make a tradeoff with a few more bytes in the built files, gaining simpler dependencies in the main files (where validations are used). Maybe its possible to reduce the final size with other changes enabled by this though. Should be worth a try.

@jzaefferer
Copy link
Contributor

We should try this asap, as it will simplify most future development.

@jzaefferer
Copy link
Contributor

I don't know why you labelled this a "question". Looking at newer PRs I still think this should get more attention, before we add more features.

@rxaviers
Copy link
Member Author

Hello Jörn, this has been labeled as question, because of the concerns of the description. In other words, we need to check what the real simplification will be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants