search

wooorm / xdm

Just a *really* good MDX compiler. No runtime. With esbuild, Rollup, and webpack plugins
http://wooorm.com/xdm/
MIT License
593 stars 18 forks source link

Fix generated JSX for custom elements #106

Closed mdynnl closed 2 years ago

mdynnl commented 2 years ago

Related-to: https://github.com/remarkjs/remark-math/issues/72 https://github.com/wooorm/xdm/commit/ddcebdbcb577015a8930b310ee7a3ba045f1c24a

mdynnl commented 2 years ago

Yes, thought about a new function that would support jsx ids, but not confident enough to do that. I'll try again with the above code.

wooorm commented 2 years ago

Does that work? Need help?

mdynnl commented 2 years ago

Yes, worked after a little fix contRe.test(String.fromCharCode(code)) to esCont(code). Is that correct? But something came up and didn't push. I'm back now. Will push if the above is correct.

mdynnl commented 2 years ago

Or maybe it also works if contRe is imported from esutil.

wooorm commented 2 years ago

Ah yes I made a mistake there. I meant cont, and calling it!

mdynnl commented 2 years ago

I'm behind a proxy and can't run the full test. Only ran test/core.

mdynnl commented 2 years ago

Ah yes I made a mistake there. I meant cont, and calling it!

And I thought it was intentional and a test to me ^_^

mdynnl commented 2 years ago

Thanks for pointing that out again. I got it but can't meet the coverage for branches. Still at 99.75%.

Edit: c8 is right. JSXMemberExpression has no child node of Literal type. That branch will never execute given the amount of the tests written including the one I added. I wonder if we still need toJsxIdOrMemberExpression. Instead construct the node manually like openingElement and it worked fine until now.

mdynnl commented 2 years ago

This was changed from manual node construction to function in https://github.com/wooorm/xdm/commit/62e6f30af2fb4d2fca73c55ca8d605615c6d27cb

Reverting that change would still work fine with dev errors, isn't it?

mdynnl commented 2 years ago

Oh, should have pushed anyway. I was waiting for your reply. Thanks for dealing with this.

wooorm commented 2 years ago

Thanks Nyi Nyi. I implemented the passing and receiving of identifier checks. It is indeed impossible to hit 100% coverage normally. That is okay. It is an edge case that shouldn’t occur, similar to edge case we are currently solving. By throwing an error there we can ensure that in the future, if something weird gets passed, we can solve it earlier due to the error message.

mdynnl commented 2 years ago

Sorry if I caused more problems rather than solved. First time contribution. But I learnt something. So, thanks.

wooorm commented 2 years ago

Released, thanks for your work!

No problem. That’s only natural. The second time will be easier, and so on! ;)