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

feat(plugins): inject API on window.__TAURI__ #383

Merged
merged 14 commits into from
May 23, 2023
Merged

Conversation

lucasfernog
Copy link
Member

No description provided.

@lucasfernog lucasfernog requested a review from a team as a code owner May 21, 2023 14:59
@lucasfernog
Copy link
Member Author

Note that this does not read the tauri.conf.json > build > withGlobalTauri option. It would complicate the plugin code too much for such a small JS eval, I think we could remove that option instead.
To keep it, we would need to add an API to allow the plugin to extend the JS init script in the setup hook (since that's when the config is available for the plugin).

@FabianLars
Copy link
Member

I don't know if that's what you mean with your commend but i'd prefer to make the injection optional somehow. If the user uses more than one or two plugins this can lead to a pretty big binary size increase, at least when the larger plugins are used. Especially since we'll probably always add new plugins.

Also, if we keep some kind of global config (a non-global config would be fine too imo) we could inject the "main" tauri code in tauri itself and remove it from the plugins, because it looks like that at least the smaller plugins are 90% just tauri's ipc code.
-> If that deduplication is possible i think the binary size i'm concerned about wouldn't be that much of an issue (still something to think about though) but didn't we also count this as a small security implication at some point?

@lucasfernog
Copy link
Member Author

Currently the global script is always embedded in the binary. To get around that, we'd need to use feature flags or do something clever with the build scripts. Parsing the Tauri config from a plugin build script is a big problem because we can't know the path to the file (without relying on the CLI to be used).

Each plugin script increases almost 5KB in the binary.

@FabianLars
Copy link
Member

Hmm, right. Maybe we should make it a setting on the Builders? Or since most plugins only use an init function, a separate init_with_global_tauri may not be such a big change but still a bit meh 🤔 idk if that'd be overkill

maybe just go with the current approach for now and think about it a bit over the remaining alpha time.

@lucasfernog
Copy link
Member Author

We can't reduce the binary size by adding runtime options for this, it'll only speed up the app load. For size optimizations we need to resolve it at compile time.

@lucasfernog lucasfernog linked an issue May 21, 2023 that may be closed by this pull request
@FabianLars
Copy link
Member

Right, of course... Sunday brain lag 😂

@FabianLars
Copy link
Member

we'll probably have to rethink this with the new ipc again anyway, but what about using window.__TAURI_INVOKE__ instead of the npm package import? That results in a 70% size decrease in the window-state plugin for example.

I feel like i'm overthinking this...

@lucasfernog
Copy link
Member Author

You mean that instead of import { invoke } from "@tauri-apps/api/tauri"? Damn that's a lot.

@lucasfernog
Copy link
Member Author

Ahh I see, that happens because we're not distributing the .ts files on the API package (I think some bundler fails to compile if we do). Changing to __TAURI_INVOKE__ does help us except for the notification, updater and upload plugins.

@FabianLars
Copy link
Member

i may have had another intrusive thought 😅 Do you think it'd be viable to deprecate the mirrors in favor of this? Once the npm packages are out i'd imagine most will use these, or those who use withGlobalTauri today will already use this PR.
The remaining 0.5% that use the git dependency to be able to lock it to a commit could switch to this api, or use the whole repo as a dependency with messed up import paths.
The ones that need it because of unpublished fixes, which hopefully gets rarer with the new plugin approach (more frequent releases) could also switch to this PR, use the whole repo as a dep, or use that gitpkg thingy.

@amrbashir
Copy link
Member

my 2 cents is that we should respect withGlobalTauri option and try to minimize duplication of it by relying on window.__TAURI__ api when building typescript for withGlobalTauri and import from @tauri-apps/api when building to publish.

@amrbashir
Copy link
Member

As for mirros, I don't quite like'em so I agree with anything that helps deprecate them.

@lucasfernog
Copy link
Member Author

i may have had another intrusive thought sweat_smile Do you think it'd be viable to deprecate the mirrors in favor of this? Once the npm packages are out i'd imagine most will use these, or those who use withGlobalTauri today will already use this PR. The remaining 0.5% that use the git dependency to be able to lock it to a commit could switch to this api, or use the whole repo as a dependency with messed up import paths. The ones that need it because of unpublished fixes, which hopefully gets rarer with the new plugin approach (more frequent releases) could also switch to this PR, use the whole repo as a dep, or use that gitpkg thingy.

Tbh I hate this global window API thing.. I only readded it because Amr said it is required for now for WASM consumers (I hope we can provide actual WASM bindings in the future with tauri-bindgen). Using this approach you lose type check, autocomplete etc, it sucks.

my 2 cents is that we should respect withGlobalTauri option and try to minimize duplication of it by relying on window.__TAURI__ api when building typescript for withGlobalTauri and import from @tauri-apps/api when building to publish.

I don't think there's an easy way to do this unless I'm missing some dark magic with Rollup.

@amrbashir
Copy link
Member

amrbashir commented May 22, 2023

I was thinking writing the same code twice, one that uses the npm package and one that uses the global tauri object, but maybe we could use something like this https://github.com/aMarCruz/rollup-plugin-jscc#usage to reduce the duplication

@lucasfernog
Copy link
Member Author

lucasfernog commented May 22, 2023

I couldn't get jscc to work 100% because of aMarCruz/rollup-plugin-jscc#13
At this point I think it's easier to just go with __TAURI_INVOKE__ anyway.

@amrbashir
Copy link
Member

Alright, lets just go with this and think about improving it some other time.

@lucasfernog lucasfernog merged commit b131bc8 into v2 May 23, 2023
@lucasfernog lucasfernog deleted the feat/global-js branch May 23, 2023 17:20
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.

[feat] withGlobalTauri for plugins
3 participants