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

Extract presence viewer web code only for Swift #7

Merged
merged 11 commits into from
Jan 14, 2025
Merged

Conversation

bplattenburg
Copy link
Member

@bplattenburg bplattenburg commented Jan 3, 2025

Instead of the original approach, I'm now extracting only the JS and HTML web content to here, and keeping everything else in DittoSwiftTools. This should massively simplify the cutover and avoid potentially painful breaking changes for end users.

As such, this will fully replace the current way I was testing, and will result in a 2.0.0 release of this package.

To see this change in action, see getditto/DittoSwiftTools#166

Part of CXTOOLS-337
Part of CXTOOLS-358


public struct DittoPresenceViewerResources {
// These public accessors let consuming packages (e.g. DittoSwiftTools) use the bundled resources (JS & HTML) from this package
public static let webDistDirURL = Bundle.module.bundleURL.appendingPathComponent("Resources/dist")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caveat: I don't fully understand how this is used in practice in the swift tools project.

I noticed that Resources/dist dir you are referencing doesn't exist - should it?
Also it's not a change that is part of this repo but I can see that the HTML/JS/CSS files are also located in Shared/ top level dir - is it intentional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To see this in use, you can take a look at getditto/DittoSwiftTools#166.

Resources/dist isn't part of whats here, but I think its generated or used at runtime. For that, I moved the logic we used to have in DittoSwiftTools here and left it generally unchanged.

Yes, there's currently duplication, but we can't fix that until we're able to use swift-tools 6.0 everywhere due to swiftlang/swift-package-manager#6982. Once this is merged I'll open a draft PR to remove the duplication & update swift tools but that will likely have to sit for a while until we're ready for everything to use Swift 6.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to leave a draft PR (#8) open to track removing this duplication long-term whenever we're able to move to swift-tools 6.0

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here is this used in the POS demo app, showcasing no breaking changes for consuming apps: getditto/demoapp-pos-kds#69

@bplattenburg bplattenburg requested review from a team and zmarkan January 9, 2025 16:49
@bplattenburg bplattenburg merged commit d7e8f40 into main Jan 14, 2025
@bplattenburg bplattenburg deleted the BP/box branch January 14, 2025 15:54
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.

3 participants