Skip to content

refactor: do not pass entire config from CLI to core, send patch instead#4598

Merged
lucasfernog merged 1 commit intodevfrom
fix/tauri-config
Jul 6, 2022
Merged

refactor: do not pass entire config from CLI to core, send patch instead#4598
lucasfernog merged 1 commit intodevfrom
fix/tauri-config

Conversation

@lucasfernog
Copy link
Member

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

@lucasfernog lucasfernog requested a review from a team as a code owner July 5, 2022 23:09
@lucasfernog lucasfernog requested a review from FabianLars July 5, 2022 23:09
@lucasfernog
Copy link
Member Author

This PR changes the TAURI_CONFIG environment variable to represent the config JSON from the CLI's config argument instead of the entire JSON. This prevents issues when the CLI is newer than the core crates, and sends a config value that is unknown to the application, resulting in a build error.

@FabianLars
Copy link
Member

tbh i expected this to break something but everything i tested (same version,cli newer, cli older) worked lol, except of course if you add conf properties one side doesn't know of. Because of that we mayyy wanna consider improving the tauri-build panic message in the future, because this:

 [...]
  --- stderr
  thread 'main' panicked at 'error found during tauri-build: Error("unknown field `kekw`, expected `bundleMediaFramework`", line: 0, column: 0)', C:\Users\Fabian-Lars\dev\tauri\core\tauri-build\src\lib.rs:172:5
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...
       Error failed to build app: failed to build app

not only scares non-rust users but nobody really understands/knows that this means that they need to update their crates.
But that's not important for now imo, just wanted to mention it so we have it in mind.

@lucasfernog
Copy link
Member Author

I'll adjust the error message in a separate PR.

@lucasfernog lucasfernog merged commit fa028eb into dev Jul 6, 2022
@lucasfernog lucasfernog deleted the fix/tauri-config branch July 6, 2022 12:29
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.

2 participants