alpheios-project / embed-lib

Alpheios Embedded Library
Other
6 stars 0 forks source link

Move options to data models #123

Closed irina060981 closed 3 years ago

irina060981 commented 3 years ago

It is update for this PR alpheios-project/alpheios-core#551 I was to update Embedded code to import direct dependencies for both - components and data-models.

I didn't add a link for data-models for cdn, because I don't know how to get it. Would be glad to get help about it :)

balmas commented 3 years ago

It is update for this PR alpheios-project/alpheios-core#551 I was to update Embedded code to import direct dependencies for both - components and data-models.

I didn't add a link for data-models for cdn, because I don't know how to get it. Would be glad to get help about it :)

we will need to add npm packaging and release steps for data-models to alpheios-core

balmas commented 3 years ago

or else have components expose Options. I am worried we will run into the same old build conflicts we used to have if we import them both separately since components includes data-models

kirlat commented 3 years ago

or else have components expose Options. I am worried we will run into the same old build conflicts we used to have if we import them both separately since components includes data-models

I think the data-models should export classes, but components should reexport data-models classes too. So if we need just data-models, we put them as a dependency, and export data from data-models. But if we need the bigger components package, we should import the components package only and import all classes from it.

I'm afraid that if we will import both data-models and components webpack won't be smart enough to understand it's a duplicated code. As a result, we'll end up with same classes included twice. What's worse, we might have issues with Symbols. In order symbols for the language to be equivalent, it mist be exactly the same Symbol instance in all the places (i.e. instantiated in one place within the bundle). The symbols won't match each other otherwise and we'll end up with symbol for Latin not being equivalent with itself (as those might be instantiated in two different places).

Sorry I've missed this one.

kirlat commented 3 years ago

I think Options should be exposed from components in the same way as Logger

Agree with that.

irina060981 commented 3 years ago

I believe we have a conversation about import/export inside monorepo to use it sometimes as not a monorepo . I think that we should have some a straightforward way to work with it as with separate packages (as I need in alignment editor). May be in future we would need to use them without components in some other applications.

But anyway let's do it through components here - and not through components inside alignment-editor.

I believe we would have a new iteration of this conversation - when I would try to use alpheios-core to activate Alpheios Tools on the page with Alignment Editor.

Ok, then I should close the PR, it doesn't need any changes then.

kirlat commented 3 years ago

I believe we have a conversation about import/export inside monorepo to use it sometimes as not a monorepo . I think that we should have some a straightforward way to work with it as with separate packages (as I need in alignment editor). May be in future we would need to use them without components in some other applications.

I tend to look at our current approach as following. We have small atomic packages (the ones that do not depend on other packages from alpheios-core): data-models, l10n, etc. We also have bigger packages that include atomic packages and adds more functionality: client-adapters, inflection-tables. We also have the "ultimate" package that includes everything: components.

So if we want to use something from just the l10n, we list this package as a dependency and import classes from it. If we need something bigger, such as client-adapters, why do we need to have a dependency on both client-adapters and l10n if l10n is already included within the client-adapters (we can't avoid that with the current build process)? It is simpler to have a dependency on client-adapters and import from there.

I think I see the benefits in approach you've described: it's better to have specialized modules each responsible for a specific area. I'm not sure what would be the most efficient approach for the webpack builds though. Webpack 5 introduces a "modules federation" that allows for components not to include all dependence packages into a monolithic bundle, but rather have atomic bundles like data-modules, l10n combined within a common container yet stay independent. It is a proprietary standard though and it has yet to be researched what exact advantages will it bring and I'd rather not to change a build process right now.

But the biggest limitation at the moment are symbols: all symbols that must be equal shall originate from exactly the same place. That makes the current approach, on my opinion, the most reliable one.

irina060981 commented 3 years ago

But the biggest limitation at the moment are symbols: all symbols that must be equal shall originate from exactly the same place. That makes the current approach, on my opinion, the most reliable one.

If to be honest I don't think that is really a good way to use Symbols as unique values through several different packages. Because we try to use them as global constants not in a global scope (not in a global registry), that's why we have problems with it.

I believe that Symbols should be mostly use for iterating through the limited list of them. But we using Symbols for checking equality like they are strings or other numerable variables, that are compared between realms by value. But by there sence Symbols are created not to be equal in different realms in oposite, that's why they shouldn't be compared by equality.

kirlat commented 3 years ago

If to be honest I don't think that is really a good way to use Symbols as unique values through several different packages. Because we try to use them as global constants not in a global scope (not in a global registry), that's why we have problems with it.

Agree with that. We already have a symbol-less Language class defined in data-models, but it will take a while to switch all libraries to use it. Once it's done, however, will get much more freedom in how the code can be built into packages. I think the Language is the only thing that is holding us back now. We should use the new Language class whenever possible as a replacement for the existing one, and that should make our life easier at the end 🙂.

What do you think? Would the new Language class being a suitable replacement for the current language info implementation?

irina060981 commented 3 years ago

I think that whithout Symbols it would be much more easier to support :) And I think new Language class is good for this.