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

Update Edit API handles namespaced attributes poorly #132

Open
ca-d opened this issue Aug 15, 2023 · 4 comments
Open

Update Edit API handles namespaced attributes poorly #132

ca-d opened this issue Aug 15, 2023 · 4 comments

Comments

@ca-d
Copy link
Contributor

ca-d commented Aug 15, 2023

The Update event could use an update, since the current handling of namespaced attribute updates is both cumbersome and - in one edge case - insufficient.

Currently attributes are grouped by (unprefixed) name and attach an extra namespaceURI parameter to the value:

const update: Update = {
  element,
  attributes: { attrName: { namespaceURI: 'https://example.org/myNS', value: 'attrValue' } }
}

The cumbersome aspect is that namespaceURI is quite long to type.

The functionally insufficient aspect is that this disallows setting two different attributes from two different namespaces if they happen to be named the same, e.g. if I wanted to simultaneously set both the svg:x attribute and the sxy:x attribute from the SVG and IEC 61850-6 Annex C.1 namespaces, respectively.

I suggest changing this part of the API by separating namespaced from non-namespaced attributes in the update event like so:

const updateV2: UpdateV2 = {
  element,
  attributes: { attrName: 'attrValue' } // non-namespaced attrs only
  attributesNS: { 'https://example.org/myNS': { attrName: 'attrValue' } }
}

resulting in a new type

export type UpdateV2 = {
  element: Element;
  attributes: Partial<Record<string, string | null>>;
  attributesNS: Partial<Record<string, Partial<Record<string, string | null>>>>;
};

Since this project is still in alpha, I think the new type could actually replace the current Update type without too much trouble, which seems preferable to me. If this does not happen until the first stable version of open-scd-core is released, I would then tend more to adding a new UpdateV2 type in addition to the old one and consequently supporting the deprecated legacy API til kingdom come.

@trusz
Copy link
Member

trusz commented Aug 18, 2023

If I understand correctly an example with more attributes could look like this?

updateV2: UpdateV2 = {
  element,
  attributes: { attrName: 'attrValue' } // non-namespaced attrs only
  attributesNS: { 
    'https://example.org/myNS': { 
       attrName1: 'attrValue1',
       attrName2: 'attrValue2', 
    },
    'https://example.org/userNS': { 
       name: 'John',
       age: '42', 
    }
  }
}

probably one object would look weird right? so just one attributes object and an empty key string would be the non-namespaces attribues

updateV2: UpdateV2 = {
  element,
  attributes: { 
    '': { // non-namespaced attrs only
      attrName: 'attrValue'
    } 
    'https://example.org/myNS': { 
       attrName1: 'attrValue1',
       attrName2: 'attrValue2', 
    },
    'https://example.org/userNS': { 
       name: 'John',
       age: '42', 
    }
  }
}

@trusz
Copy link
Member

trusz commented Aug 18, 2023

which alternative names have you considered?
e.g.:

  • attributeWithNameSpace
  • nameSpacedAttributes
  • etc...

@ca-d
Copy link
Contributor Author

ca-d commented Aug 19, 2023

probably one object would look weird right? so just one attributes object and an empty key string would be the non-namespaces attribues

Yeah empty would look weird I think. I liked attributes and attributesNS because it mirrors the Web API's naming with setAttribute and setAttributeNS and is therefore a bit mnemonic for the Web API methods that we actually invoke upon receiving the events in question.

@trusz
Copy link
Member

trusz commented Aug 19, 2023

Oh yes. That makes sense!

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

2 participants