Fix empty path component in route registration

[marius-se]

Currently all routes registered through this library will have an empty path component:
Example:
"/users" got split into ["", "users"]. With this PR "/users" will now be split into ["users"] only!

[MahdiBM]

@swift-server-bot test this please

[MahdiBM]

LGTM 👌

[MahdiBM]

@0xTim I probably don't have access to ask the bot to run the tests 🤔

[0xTim]

@swift-server-bot test

[0xTim]

@swift-server-bot please test

[0xTim]

@swift-server-bot test this please

[0xTim]

@tomerd how do we trigger CI? 😅

[marius-se]

I think @swift-server-bot is oof? 😆

[tomerd]

@swift-server-bot test this please

[0xTim]

@marius-se as the error says - do you want to fix the Equatable conformance in RoutingKit?

[MahdiBM]

I'm not at my desk, but as I see this, you could either not use the equtable conformance or use #if $Retroactive to use @retroactive where the feature exists, and not use that when it doesn't like in 5.9. Both are fine by me.

[MahdiBM]

Correction: the spelling is #if $RetroactiveAttribute.

[0xTim]

We should fix the underlying issue since this is adding the conformance - we shouldn't be extending types we don't own to protocols we don't own. I didn't realise this was a new error but it's definitely a valid one, and one that Vapor is going to get screwed by 😅

[MahdiBM]

@0xTim I was also worrying about that when I first reviewed, but @marius-se did it the "fine" way which is to only add it to Tests. That's why I'm fine with it existing. Totally removing it is also fine and might be slightly better as well.

[marius-se]

Sooooo... which option do you want me to proceed with 😆

1. Fix in Routing Kit
2. Fix in test (adding retroactively thing)
3. Remove conformance and manually compare

(I'm happy with all three solutions)

[MahdiBM]

I think we can proceed with the third option and it should be fine 🙂
maybe something like:

let path = try XCTUnwrap(app.routes.all.first?.path)
XCTAssertTrue(path == ["hello", ":name"], "Unexpected path: \(path)")

But implementing == as a global function, not as a Equatable conformance to the type:

func == (lhs: [PathComponent], rhs: [PathComponent]) -> Bool {
    lhs.count == rhs.count &&
    zip(lhs, rhs).allSatisfy { 
        /// Logic for `$0 == $1`, the same as you wrote for the Equatable conformance
    }
}

<Untested code alert, might not compile as-is>

[MahdiBM]

Or actually i didn't notice Tim recommending to fix the conformance in RoutingKit

do you want to fix the Equatable conformance in RoutingKit?

That's fine by me too.
Generally all 3 ways are fine. Fixing the conformance in RoutingKit or using the third way are probably better though.

[0xTim]

You can hack it in for the tests, but I see no issue with adding it directly to RoutingKit as well

[MahdiBM]

@marius-se I think this should not be much of a problem now that the RoutingKit PR is merged.

[MahdiBM]

@swift-server-bot test this please

[MahdiBM]

@0xTim 🫣

[0xTim]

@swift-server-bot test this please

[0xTim]

@swift-server-bot please test

[0xTim]

@MahdiBM feel free to merge and release once CI is green