gbv / cocoda

A web-based tool for creating mappings between knowledge organization systems.
https://coli-conc.gbv.de/cocoda/
MIT License
39 stars 5 forks source link

Move util to a separate package #543

Closed stefandesu closed 4 years ago

stefandesu commented 4 years ago

When starting to think about refactoring for version 1.3.1, I decided that it would make sense to refactor and split off the util in Cocoda. So I created cocoda-utils and moved some util functions there, some to jskos-tools (where it made sense). I also adjusted some of the methods, resulting all in all in lots of changes in Cocoda. These are currently in the branch utils-refactor and deployed to https://coli-conc.gbv.de/cocoda/test/. I've been testing extensively today and only found minor issues not related to this refactor (#539, #540, #541, #542).

It would be good if someone else could take a look at it as well (@nichtich). I've had some issues with reactivity when changing the interface language, so please try that (should be working now though). Also take a look at the console for unusual error messages.

I will probably merge this on Tuesday or Wednesday, depending on the feedback.

stefandesu commented 4 years ago

I adjusted cocoda-utils to be a singleton which means the following: We can use cocoda-utils even in modules that we want to separate from Cocoda. The way that Node.js works is that it will reuse the same object if cocoda-utils is imported multiple times which in this case works in our favor.

So if we, for example, separated ItemName from Cocoda (which I eventually plan to do), we could just import and use cocoda-utils like normal, and if it's used inside of Cocoda, it will still allow reactivity to work because Cocoda is ensuring that cocoda-utils has the right reference to the language priority list.

nichtich commented 4 years ago

I don't fully understand the separation of cocoda-utils and jskos-tools. Most of the former could be moved into the latter, no?

stefandesu commented 4 years ago

I don't fully understand the separation of cocoda-utils and jskos-tools. Most of the former could be moved into the latter, no?

I understand that and I have thought about putting everything into jskos-tools (that's why some stuff actually went into jskos-tools). I think part of the reason that I've decided to separate it is that cocoda-utils has some methods which have nothing to do with JSKOS (but are used a lot in Cocoda) and the JSKOS related methods are mostly UI-related. Also, jskos-tools is already at a point where I didn't want to decide to just put lots of new methods in there. Let's talk about this later, maybe it makes sense to put everything into jskos-tools.

stefandesu commented 4 years ago

Soooo, I moved all the JSKOS-related methods into jskos-tools and adjusted Cocoda in the utils-refactor branch. Please check the test deployment for any unusual behavior regarding language handling or console error messages:

https://coli-conc.gbv.de/cocoda/test/?fromScheme=http%3A%2F%2Fbartoc.org%2Fen%2Fnode%2F1940&from=http%3A%2F%2Fwww.wikidata.org%2Fentity%2FQ43518

Note: Currently, I'm getting error messages from the ZBW API, this is unrelated to these changes.

stefandesu commented 4 years ago

Another thing: cocoda-utils now only has three methods left:

Do we really need a separate package for that? We could also just put it into the global mixin to be used in the components and get rid of the separate package. @nichtich

stefandesu commented 4 years ago

Everything merged and seems to be fine!

stefandesu commented 4 years ago

I also archived cocoda-utils.