refactor: replace kuchikiki with dom_query - #14959
Conversation
Package Changes Through 91888fbThere are 9 changes which include tauri-macos-sign with patch, tauri-build with patch, tauri with minor, tauri-bundler with patch, tauri-cli with patch, @tauri-apps/cli with patch, tauri-runtime-wry with minor, tauri-runtime with minor, tauri-utils with minor Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
Legend-Master
left a comment
There was a problem hiding this comment.
Thanks for working on this
Could we preserve the non mut version of those functions? Asking because it is technically going to break people updating tauri-utils without bumping tauri (we don't fully follow semver on tauri-utils but unfortunately we didn't mark tauri and other dependencies to only take a smaller version range yet)
Also we'll need a change file here and the wry one, and this one will need to wait for the wry one to go through first
That is a bit tricky to do. We'd have to make a wrapper for the document that uses interior mutability. Is that something you want? |
Doesn't |
Yep, I just discovered that as well! Sorry for the bad first iteration. I've revised this to not be mutable but it is still a breaking change because we used to have |
|
Not sure what to do about the CI failures regarding |
Yes, I'm happy as long as old tauri still work with the new tauri-utils though 😂 cc @FabianLars any thoughts on this? To me I think we should probably mark the entire tauri-utils crate as if it's pre-1.0, meaning 2.5 -> 2.6 will contain breaking changes and adjust our crates to rely on minor versions. Not sure if we should do this before 3.0 though
No worries, that's a cache problem for quite some time now |
problem is that we re-export tauri-utils in tauri as tauri::utils
i removed the cache, next run should work. |
If we decide to do that, we would also need something like we did for Or we should re-think about if we should have an internal util crate or something in v3 |
|
yeah, something like this i guess. or we make a different design choice for the utils crate. eg no breaking changes (builder pattern or non_exhaustive?) and no third party types in the api and whatever else we can think of. but yes, we talked about splitting the utils crate up a few times as well |
Yeah that means any breaking change to If you want to be strict about semver, my suggestion would be:
What is the current policy here? Will this have to wait until Tauri v3? |
breaking changes in struct fields are fairly common but this here is a bit much for 2.x i think |
Because we are exposing a type with a different API? What if we create a wrapper around |
|
if we could mimic the api in a way that would not break "old" tauri-codegen and tauri versions that somehow use newer tauri-utils versions that would be ideal. I assume virtually nobody uses the html-manipulation feature directly so i don't really care about anything else (including the use in tauri-cli) not sure how reasonable/realistic that wish is |
|
We can mimic it such that zero code changes are required when you upgrade both. But you still need to upgrade both because they are just not the same type. If you only upgrade I was under the impression that all tauri crates need to be bumped together anyway always. There is one approach that we can take if you want to be fully backwards-compatible:
Let me know what you prefer. |
|
Looking at the usage, mimicking the methods can be quite hard to maintain since we'll need to wrap them with our own structs Feature flags can work, but it's also quite hard, we'll need a new default feature set and enable the old feature for If the dependency issue of kuchikiki isn't critical, I am leaning towards delaying this to v3 And in v3, I would really like to bump all tauri crate minor versions together, and make the dependency rely on minors (e.g. (or alternatively, change up the way we release things by bumping major versions and use a 'forth' digit to indicate major feature/change releases (like 301.5.2 to indicate major version 3, breaking change 1, minor 5, patch 2) 😂 https://antfu.me/posts/epoch-semver) |
I don't think we need to? That is entirely backwards-compatible and allows anyone to remove |
9bbd2d4 to
50fb8a9
Compare
|
Let me know what you think of the latest version. If I am not mistaken, this is fully backwards-compatible. Feature-naming subject to bike-shedding of course, I just wanted to demonstrate the concept. |
|
i for one could totally live with this approach :) |
Cool! I'll fix the clippy warnings and then test in our repo to ensure the dependencies do go away. |
|
Okay, CI should be passing now. I've patched in this branch into my repo and it seems to work. The thing though is, all plugins will need to be updated to no longer use the For now, I've unrolled the |
Actually, it is only the |
I ended up introducing it because there is also a dependency on the |
|
Juggling feature flags is not exactly easy, it feels to me that this is too much effort for upgrading a non-critical dependency here. And because of the way features work, it is possible for projects to end up with both branches pulled in and compiled. That being said, I'm not opposed to this if that's not me working on it 😂 Thanks for the effort! |
|
Will wait for new wry and dom query versions and then we can merge this (done as of And with the chance we got, we might want to also clean up as many |
Splitting it up into tauri-utils and tauri-internals or something is stupid right? |
In my humble opinion, anything "utils" is always a code-smell because it always end up being a catch-all bucket for all kinds of functionality and causes an entangled mess. Of course, it comes with its own bit of complexity to create a So no, I don't think it is a stupid idea. I think it is actually a great idea IF the boundaries of the crates are designed around their functionality, i.e. |
|
yeah i think we can all agree that tauri-utils contains way too much 😅 but perhaps it's not the time to split it up and simply wait for v3 (iirc getting the config part out of utils was also very tricky?) |
|
An internal crate sounds good to me, but in the other hand, I never even realized that the tauri-utils crate is actually a public crate (and tauri even re-exports it) until recently say half a year ago 😂 I don't know how common is a non-semver following internal crate practice in the rust world, but it seems to be something quite common for the npm packages, like In the other hand, some other packages/crates choose to 0.x for internal packages/crates that does follow semver Both approaches have their own trade offs though
Can't remember the details anymore, I think it was mainly about the ACL and other things being shared by both the config and tauri APIs like |
|
i'd still follow semver for external crates just to not make cargo's life harder than necessary but imo we don't need to follow tauri's version e.g. we can keep it in 0.x or like chrome on v145+ 🤷 That would also make some de-duplication possible assuming that not every single change we have there is breaking |
I think that is the root of the issue here. utils crates are fine as long as you are not making them part of your public API. They are "public" in the sense of everyone can depend on it but the versioning doesn't matter then, right? It wouldn't matter if |
91888fb
Legend-Master
left a comment
There was a problem hiding this comment.
Thanks again for the patience @thomaseizinger !
* test: add more unit-tests for `html` module * refactor: remove html dependencies from `tauri-cli` * feat: introduce `html-manipulation-2` feature * Remove deprecation * Use new feature flag * Unroll `build` feature * Introduce `build-2` feature * Reduce diff * Use `build-2` in more places * Add docs * Refactor `inject_script_hashes` * Refactor `with_head` * Rename serialize and parse functions * Add changes file * Remove unused function * Update changelog * Remove test * Update wry * Add todo comments we don't have the git blame data in html2, better do it now or never find it again * refactor `with_head` to `ensure_head` * Remove unused casts * Avoid using format to construct html elements which has the potential to get injected * Feature gate `inline_isolation` * Keep old prepends appends * Fix `inline_isolation_replaces_src_with_content` test * End meta tag * Mirror test to old html module * Use back to `append_html` for csp and link issue * Try out dom query main branch * Use nodes instead to avoid an extra clone * Use wry 0.54.4 and dom_query 0.27 * Mark stability * Remove `PatternObject` --------- Co-authored-by: Tony <legendmastertony@gmail.com> Co-authored-by: Tony <68118705+Legend-Master@users.noreply.github.com>
* test: add more unit-tests for `html` module * refactor: remove html dependencies from `tauri-cli` * feat: introduce `html-manipulation-2` feature * Remove deprecation * Use new feature flag * Unroll `build` feature * Introduce `build-2` feature * Reduce diff * Use `build-2` in more places * Add docs * Refactor `inject_script_hashes` * Refactor `with_head` * Rename serialize and parse functions * Add changes file * Remove unused function * Update changelog * Remove test * Update wry * Add todo comments we don't have the git blame data in html2, better do it now or never find it again * refactor `with_head` to `ensure_head` * Remove unused casts * Avoid using format to construct html elements which has the potential to get injected * Feature gate `inline_isolation` * Keep old prepends appends * Fix `inline_isolation_replaces_src_with_content` test * End meta tag * Mirror test to old html module * Use back to `append_html` for csp and link issue * Try out dom query main branch * Use nodes instead to avoid an extra clone * Use wry 0.54.4 and dom_query 0.27 * Mark stability * Remove `PatternObject` --------- Co-authored-by: Tony <legendmastertony@gmail.com> Co-authored-by: Tony <68118705+Legend-Master@users.noreply.github.com>
This PR harvests the changes we have landed upstream in tauri-apps/tauri#14959 and allows us to remove a lot of outdated dependencies from our dependency tree that got pulled in by the `kuchikiki` dependency which is more or less unmaintained.
This PR removes the dependency on
kuchikikifrom thetauri-cliandtauri-utilscrates. It is still being pulled in bywrybut that is not part of this repository and needs to be dealt with separately.Resolves: #14958