search

SolidOS / mashlib

Solid-compatible data mashup library and Data Browser
https://solidos.github.io/mashlib/dist/browse.html
MIT License
71 stars 21 forks source link

Try move auth #150

Closed timea-solid closed 2 years ago

timea-solid commented 2 years ago

This is the PR for the move auth from solid-ui to solid-logic.

timea-solid commented 2 years ago

@jeff-zucker When you have time take a look at the PR, particularly the file static/browse.html and check how repos are used from mashlib now. UI is directly used and cannot be exported from panes.UI anymore. Here in mashlib 'panes' was the entire mashlib so it was wrong anyway.
panes.UI can still work if I put the UI to be exported from solid-panes back -> see this PR too: https://github.com/solid/solid-panes/pull/337 Also 'authSession' will come from UI.authSession and not from UI.authn.authSession.
Let me know what you think.

jeff-zucker commented 2 years ago

I'm sorry, I really do not like these changes. There are dozens and dozens of places across many libraries that use panes.UI and panes.UI.authn. Introducing a new way of exorting in addition is fine, but I strongly object to making all of these backward-incompatible changes. If these are instituted, it will really have to be a major version release.

timea-solid commented 2 years ago

Follow up on the usage of the global variables: I found out in solid-ui in the openHrefInOutlineMode function we make use of window.panes -> panes here is solid-panes. This introduces a cyclic dependency in our repo -> needs to get refactored out. Consequence: make sure panes is added to the window global variable. also: window.panes.getOutliner, window.panes.UI

jeff-zucker commented 2 years ago

Are you talking about how these things are addressed internally or how they are exported? Because exporting window.panes but not panes will break everything. Not sure if that is what you are suggesting, but if so that will break backwards compatability.

timea-solid commented 2 years ago

Are you talking about how these things are addressed internally or how they are exported? Because exporting window.panes but not panes will break everything. Not sure if that is what you are suggesting, but if so that will break backwards compatability.

@jeff-zucker I am talking about this and next lines of code in solid-ui: https://github.com/solid/solid-ui/blob/707601b18fc26917752ae0150bdc55058a34312e/src/widgets/buttons.ts#L987 -> this is really really bad.

jeff-zucker commented 2 years ago

So to whom is the advice "use window.panes not panes" addressed to and in what circumstance would it be used?

jeff-zucker commented 2 years ago

Isn't that line just saying that if there is no outliner but there is a winodw.panes.outliner, use that outliner? How is that bad?

jeff-zucker commented 2 years ago

Also why do you believe that other code should create window.panes?

timea-solid commented 2 years ago

The questions above were addressed in a one on one chat. Some conclusions: