Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comments on documentation and possible bugs #75

Open
cblakeley opened this issue Dec 4, 2020 · 5 comments
Open

Comments on documentation and possible bugs #75

cblakeley opened this issue Dec 4, 2020 · 5 comments

Comments

@cblakeley
Copy link

I've been trying to build a simple SPA to exercise LDflex and familiarize myself with it in the process. Below are some comments on the documentation and other issuess I've had.

Thanks in advance for any comments or guidance on any misunderstanding on my part.

Carl B.

Documentation

Converting strings to data paths

Looking at the README at https://github.com/LDflex/LDflex, it wasn't apparent to me whether it was possible to convert a string to a data path. The examples in the README navigate data paths using dot notation, where the data path is hardcoded.

e.g. for await (const name of person.interest.label)

I wanted the facility to use a string entered into a form as the basis for a path, so the property names in the path are computed rather than being hardcoded. I found this worked:

let resolvedDataPath = subjectPath.resolve(ldfDataPath);
// resolvedDataPath = subjectPath[ldfDataPath]; // also works
// e.g.
// subjectPath.resolve('.interest.label') or
// subjectPath['interest']['label']

though it wasn't obvious from the LDflex documentation. The documentation for LDflex for Solid in section 'The any URL entry point' shows using strings as property names, but this is specific to solid.data object, whereas I was using LDflex independently of Solid.

Getting all properties of a subject

In this section:

for await (const subject of document.subjects)

it wasn't clear to me what subject should be. I assume it should be a path instance, as the following worked for me:

let srcPath = pathFactory.create({ subject: namedNode(src) });
for await (const subject of srcPath.subjects) {

Similarly with the example in section Getting all properties of a subject, for await (const property of subject.properties)

Return a single value as an async iterable?

Does LDflex have the facility to return a single value as an async iterable? It seems that you have to know beforehand whether a property path will return a single value or multiple values. The access method is different in each case, e.g:

${await person.name}

or

for await (const name of person.interest.label)

'for await...of' doesn't work with values which are not async iterables; so can't be used with single values.

Bugs?

Bug? The returned properties can include some belonging not to the given subject but to another subject in the source document.

Using
for await (const property of subjectPath.properties) {...
to request the properties of subject https://ruben.verbough.org/profile/#me I see
cc:attributionName as one of the listed properties. However in your profile, I think this only occurs as a property of <https://data.verborgh.org/ruben>.

Bug? Similarly I think the subjects
returned by for await (const subject of srcPath.subjects) {... include some which aren't immediate children of the source document (https://ruben.verbough.org/profile/)

@RubenVerborgh
Copy link
Member

Thanks @cblakeley, a small part of the reply already:

Does LDflex have the facility to return a single value as an async iterable? It seems that you have to know beforehand whether a property path will return a single value or multiple values.

'for await...of' doesn't work with values which are not async iterables; so can't be used with single values.

LDflex paths are both Promises and async iterables, so you can do:

  • await person.name to output a single name
  • for await (const name of person.name) to output multiple names
  • await person.interest.label to output a single interest
  • for await (const name of person.interest.label) to output multiple interests

Bug? The returned properties can include some belonging not to the given subject but to another subject in the source document.

Those look like two bugs indeed; could you please file them as a separate issue?

@cblakeley
Copy link
Author

cblakeley commented Dec 4, 2020

Suppose you have

let resolvedDataPath = subjectPath.resolve(ldfDataPath);

and you don't know how many values ldfDataPath will return.
In trying to just code for the generic case, using:

for await (const val of resolvedDataPath) {
  data.add(val.toString());
}

with dataPaths which return a single value, I'm seeing that the for await...of loop is entered twice. It seems you have to know beforehand whether a path will return a single value or multiple values, then choose between await person.name or for await (const name of person.name). The latter doesn't handle the case where a person has a single name, you get the same name back twice. Is it possible to enable some sort of "list mode" where a single value would be returned in an iterable, so you can just code the same irrespective of whether a property returns one or multiple values?

@RubenVerborgh
Copy link
Member

and you don't know how many values ldfDataPath will return.

That doesn't really matter.

The question is rather: what do you want as an app developer?
Do you have space for one value (for example, a UI with space for one profile pic and one name)?
Then ask for one value with await.

Are you prepared to deal with multiple values?
They ask for them with for await.

The latter doesn't handle the case where a person has a single name, you get the same name back twice.

That should not happen, could you provide a concrete example?
Might be easy to link it from https://solid.github.io/ldflex-playground/

Is it possible to enable some sort of "list mode" where a single value would be returned in an iterable, so you can just code the same irrespective of whether a property returns one or multiple values?

That's the idea indeed; let's see where it goes wrong.

@cblakeley
Copy link
Author

There's a simple React test application for exercising LDflex at https://github.com/OpenLinkSoftware/Flexpress and a demo instance at http://ods-qa.openlinksw.com/flexpress/. It's when developing this that I've come across these issues.

I've experienced problems running Flexpress with Chrome (#51), but I’ve had no problem with Firefox.

The app is a work in progress. The ‘Subject Properties’ dropdown is populated by the ‘Subject Properties’ button, but these properties currently have no effect on the listed LDflex data paths. The aim is, at some point, to extend the UI to allow an LDflex data path to be built interactively from the available properties for the chosen subject and extend the path by descending through a property to a child property.

I've raised the apparent bugs mentioned in this issue as separate issues.

Thanks for your feedback (and for LDflex - nice work!)

@jeswr
Copy link
Member

jeswr commented Dec 10, 2020

it wasn't clear to me what subject should be. I assume it should be a path instance, as the following worked for me:

let srcPath = pathFactory.create({ subject: namedNode(src) });
for await (const subject of srcPath.subjects) {

AFAIK the subject here isn't actually made use of. This branch that I am currently using for some of my applications allows the following syntax.

let document = pathFactory;
for await (const subject of document.subjects) {

The solution that is used there is a bit hacky so I haven't PR'd it https://github.com/jeswr/LDflex/blob/8dd762f1e83fd230a26b84f7319e12a0c4415950/src/PathFactory.ts#L70-L84

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants