readium / ts-toolkit

A toolkit for ebooks, audiobooks and comics written in Typescript
BSD 3-Clause "New" or "Revised" License
61 stars 9 forks source link

Navigator package and ReadiumCSS versions #74

Open JayPanoz opened 1 week ago

JayPanoz commented 1 week ago

As discovered in #73, Navigator is installing version 1 of ReadiumCSS, which is fine for apps already using the Navigator as ReadiumCSS has breaking changes in v2, but makes the package not really usable for people willing to implement their app with ReadiumCSS 2 (Playground in particular).

At the moment Playground is using a submodule of ts-toolkit and can be pointed to a branch – it actually is to test current position numbers, see #62 – and the app is deployable although it requires some extra steps on Cloudflare, but that wouldn’t really help others, I guess. And having Playground use a submodule pointing to a branch with ReadiumCSS v2 isn’t really the cleanest option either.

Not sure what the other options are and if there’s even a use case outside of Playground but I guess the problem will arise when people try to migrate from ReadiumCSS v1 to v2 and need a package of the Navigator with v2.

bluefirepatrick commented 1 week ago

I don't think pointing at a well-named submodule is a bad thing, I kind of like it as it make it easier to reference the source code, but I get why you want a npm package for it. Would it be feasible to have a different project/repo for ts-tooklit for the web/playground project? Or maybe a major version release of ts-toolkit with a tag/branch for the older version? I guess it comes down to if there is a plan for long-term support for a readium-css 1.x Navigator?

JayPanoz commented 1 week ago

I don't think pointing at a well-named submodule is a bad thing, I kind of like it as it make it easier to reference the source code

Oh yeah I mean I don't have any issue with that personally as we've been working with a submodule since the beginning and I can make it work for Playground – build is not necessarily super pretty for local dev and deployment but in the end that's entirely handled in package.json, it works and that's fine-ish.

In all cases it shouldn't be blocking as far as I'm concerned, I have an option and am not depending on a fix.

Eventually, removing the submodule further down the line to use npm packages once this is resolved is no big deal, really.

Just have to check with @HadrienGardeur whether we go this route until there's a resolution.

 I guess it comes down to if there is a plan for long-term support for a readium-css 1.x Navigator.

That's the main point of this discussion indeed. Thanks for clarifying that. 🙏

At first sight I see it as a temporary issue with a resolution that offers a migration path for the current consumers of Navigator. We know of at least one product using Navigator in production and forcing ReadiumCSS v2 would be an issue as it would break auto mode for columns, etc.

That said, maybe it's more complex than I anticipate, and I hope this issue can help confirm that.

A few weeks ago we also discussed the possibility of using something else than ReadiumCSS with @mickael-menu, and whether the mobile toolkits should offer an API for that. We couldn't really find an existing use case or request for that though, but it's worth mentioning.

That would be quite some work, should such a feature request pop up, so it would be a longer term item anyway.

@chocolatkey I guess short term the idea is to make it easier for DM so that things don't break unexpectedly, and it's easier to migrate to ReadiumCSS 2.

HadrienGardeur commented 1 week ago

Eventually, removing the submodule further down the line to use npm packages once this is resolved is no big deal, really.

Just have to check with @HadrienGardeur whether we go this route until there's a resolution.

I think that we can use a submodule in the meantime, but this should be dropped once we migrate to the Preferences API (you can also open an issue about that in the playground repo @JayPanoz).

Preferences API itself should be strictly developed for Readium CSS 2.0, it doesn't make any sense to target an earlier version for this work @chocolatkey.

De Marque should be the only organization currently using Readium CSS 1.x along with the Navigator.

chocolatkey commented 1 week ago

I agree, eventually we need to drop support for anything but the latest ReadiumCSS

chocolatkey commented 1 week ago

@JayPanoz is your intent still to get #73 merged given this discussion?

JayPanoz commented 1 week ago

@chocolatkey I guess I can close it based on these additional details.