search

anvilresearch / json-document

Fast, declarative, standards-based JavaScript (ES6) data modeling & manipulation
MIT License
13 stars 2 forks source link

Move Patch, Pointer and Mapping to external lib(s) #4

Closed dmitrizagidulin closed 7 years ago

dmitrizagidulin commented 7 years ago

If we move JSONPatch.js, JSONPointer.js and JSONMapping.js to an external lib, we will save about 15.5 kb of lib size (this is before minifying, but still). They're being packaged up in every RP bundle, but aren't used. Example output of a webpack command for oidc-rp:

...
  [14] ../_auth/json-document/lib/JSONPatch.js 5.21 kB {0} [built]
  [15] ../_auth/json-document/lib/JSONPointer.js 7.08 kB {0} [built]
  [16] ../_auth/json-document/lib/JSONMapping.js 3.21 kB {0} [built]
...
EternalDeiwos commented 7 years ago

I disagree. RP uses JSON Document as a dependency which already depends on JSON Patch as well as the Initializer and Validator classes. The Initializer and Validator are currently being reworked to account for $ref and will soon depend on JSON Pointer, which is what it was originally intended for when it was included in this library. None of these can be separated out for the reason of saving bandwidth or because of lib size.

This leaves JSON Mapping. JSON Mapping already depends on JSON Pointer and if it were separated out into an external lib, would incur the same cost that you are talking about here by requiring JSON Document just for JSON Pointer. (Disclaimer: This --- admittedly --- would be different if all of the above mentioned classes were in their own libs but see above)

These could be separated out but the result of it is it would be unlikely to provide much of a benefit in terms of bandwidth usage (best case: 3.21 kB, worst case: 0 B) and would incur significant difficulty in working with them.

At the very least I would suggest that the decision for the libs to be separated or not be deferred until JSON Document is stable.

dmitrizagidulin commented 7 years ago

I see, ok. I mean, obviously if we're using them, then the argument falls away.

The reason I brought up this issue, is that currently, we're not actually using either JSON Mapping or JSON Patch in RP or JSON Doc. Which is why I figured it's safe to factor them out.

If you guys are refactoring json doc to actually use them, then that's a different story.

EternalDeiwos commented 7 years ago

All sorted then. Closing the issue.