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

Attributes as ExpressibleByDictionaryLiteral #34

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chriseidhof
Copy link
Collaborator

This PR is an experiment, not sure if it's a good idea.

I figured that it's nice to keep the declaration order of attributes.

Previously, we were sorting the attributes alphabetically during pretty printing so that they would at least always print in the same order.

Here are a few things to think about:

  • We don't remove duplicate attributes (which should be unnecessary for HTML, unless you have duplicate custom attributes).
  • The way I made Tags.swift compile is just a way to get this out without a large diff. But it also shows a problem: if people buildup their attributes using dictionaries, this approach is not nice. We could make Attributes conform to more protocols so that it feels just like an (ordered) dictionary.
  • There are no tests for this yet
  • I'm not sure if the added complexity is worth it over a simple dict.

@robb
Copy link
Owner

robb commented Oct 12, 2021

Just thinking out loud, would this Attribute pave the way for typed attributes similar to how EnvironmentKey works?

I've been thinking about whether typed attributes are worth the complexity.

We would have to define these types in a way that they are available in Generator and Swim and maybe have a fallback key associated with AnyHashable for custom attributes.

That said, how do you find yourself using Swim? This diff should only matter to you if you use custom tags as the built-in ones in HTML already have (arbitrarily) alphabetized attributes. So the order being maintained (for non-custom attributes) is the one at the function declaration site, not the call-site.

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

Successfully merging this pull request may close these issues.

2 participants