Support for xlink?

[kristoferjoseph]

Does this patch need to make it into yo-yoify? https://github.com/shama/bel/pull/31

[yoshuawuyts]

oh interesting; is it bugging out right now? - what would need to change?

[kristoferjoseph]

o.setAttributeNS(null,"xlink:href","#"+arguments[0]) 👈 This output will throw since the namespace needs to be set to: http://www.w3.org/1999/xlink Correct output needs to be: o.setAttributeNS(http://www.w3.org/1999/xlink,"xlink:href","#"+arguments[0]) Actual Error: Uncaught DOMException: Failed to execute 'setAttributeNS' on 'Element': '' is an invalid namespace for attributes.

[shama]

Dang yeah, ideally what I'd like to do is have everything handled by bel. Using bel directly, will process tagged template strings or provide an API of the operations it would make to process the template, so this transform could use that API and all that logic stays in bel. This would also fix potential versioning issues between developed with bel@x vs using yo-yoify@x.

[kristoferjoseph]

Are you saying that instead of outputting a setAttribute string yo-yoify would instead output a call to a bel API function?

[shama]

The output code would still contain direct calls to el.setAttribute() but the transform would use the version of bel the file it's transforming is using to build those statements.

So this transform, instead of copying the logic in bel and outputting strings, it would get an array of operations from bel to write to replace in the file:

const element = bel.createElement('strong', { className: 'foo' }, [ 'waddup'])
// Regular HTMLElement as we currently get: <strong class="foo">waddup</strong>

// No idea about the interface to this but we want to use the local version of bel that the file we are transforming is using
const belCreateElementOps = require(betterResolveThisPath(transformingfilepath + '/node_modules/bel/createElementOps'))
const ops = belCreateElementOps('strong', { className: 'foo' }, [ 'waddup'])
ops = [
  "var {0} = document.createElement('strong')",
  "{0}.setAttribute('class', 'foo')",
  "var {1} = document.createTextNode('waddup')",
  "{0}.appendChild({1})"
]

Then this transform just takes those operations, maybe optimize them as a group where we can, and produces the final output code.

[kristoferjoseph]

Any progress on this?

[yoshuawuyts]

I haven't had the bandwidth to look at this; sorry

On Mon, Nov 14, 2016 at 11:49 PM kj notifications@github.com wrote:

Any progress on this?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/shama/yo-yoify/issues/27#issuecomment-260489148, or mute the thread https://github.com/notifications/unsubscribe-auth/ACWlelrTaOlfYCi9vzLMunwOdY6PuUDmks5q-OVogaJpZM4KjoCG .

[kristoferjoseph]

Only reason I ask is I am not able to minify my output currently due to this issue 😿 I can work on a PR, but based off of @shama's reply I need more clarification. Here is my understanding for how I would need to get this fix landed in the blessed way:

• [ ] Add createElementOps to bel with a PR
• [ ] Make a PR that uses that method to generate the createElement/NS setAttribute/NS output strings here in yo-yoify

Does this sound correct?

[shama]

@kristoferjoseph We could add a temporary patch if needed. As the idea above is going to take some more time.

[kristoferjoseph]

OK. I can make a PR. Thank you kindly.

On Tue, Nov 15, 2016, 8:39 AM Kyle Robinson Young notifications@github.com wrote:

@kristoferjoseph https://github.com/kristoferjoseph We could add a temporary patch if needed. As the idea above is going to take some more time.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/shama/yo-yoify/issues/27#issuecomment-260694143, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEqFFwJwOFm-fT1FzARjakYOfQsBJZEks5q-eBZgaJpZM4KjoCG .