ErrorLogger Accepts Objects

[jolanglinais]

Is your feature request related to a problem? Please describe.
Revisit ErrorLogger logic. Right now, it expects an array of errors every time a clause is parsed. This means if multiple changes cause a clause to be parsed multiple times, the same error will be added to the array multiple times and we end up with multiple of the same error.

Describe the solution you'd like
It makes more sense to use an object instead of an array, where the clauseId are keys and we update the error by clauseId, so we only ever have one error per clause.

Describe alternatives you've considered
Any alternatives are welcome.

Additional context
Related issue in TSv2

[flagoon]

I would like to take this issue. It's fine to change it to Object, but did you consider using a Set? It will prevent duplicates by default, and maybe it's logic would be easier?

[jolanglinais]

I would like to take this issue. It's fine to change it to Object, but did you consider using a Set? It will prevent duplicates by default, and maybe it's logic would be easier?

I have not, actually. @DianaLease @dselman @adriaan-pelzer would this work? I'm not that familiar with Sets, looking more into it now.

[flagoon]

I have not, actually. @DianaLease @dselman @adriaan-pelzer would this work? I'm not that familiar with Sets, looking more into it now.

I was wrong, Set is not correct way to deal with it, Map would be better. But I will start with plain Object and I will see where we will get with it.

[flagoon]

Hey, I have a problem to test my solution. I tried to link the library to my application and test it, but each time I have error connected to hooks. "Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:" I tried to debug it, and I can't see any problem. Can you help me with that? I'm using clean CRA, React 16.10.2, just created 1 component like in documentation.

[DianaLease]

Hi @flagoon . This is an issue when using npm link. You'll want to edit your webpack config as shown in here: facebook/react#13991 (comment)

However, it might make sense to connect with @Ph0tonic on their PR in template-studio-v2 here: accordproject/template-studio-v2#107. It would be great to test your solution within their branch of template-studio-v2 which passes an object to the ErrorLogger. Additionally, template-studio-v2's webpack is already set up to handle the npm link issue.

[jolanglinais]

Additionally, template-studio-v2's webpack is already set up to handle the npm link issue.

Second everything from @DianaLease, and note that template-studio-v2 is a great sandbox for development of cicero-ui.

[jolanglinais]

I believe this issue is closed, but work continues in #167