Skip to content

Conversation

@timon-schelling
Copy link
Member

@timon-schelling timon-schelling commented Aug 26, 2025

  • Why: Multiple devs where confused/annoyed by an error that came from a missing frontend dist dir.
  • Add a embedded_resources feature
  • Add a build script that sets cfg(embedded_resources) if resource dir exists
  • Emits warning when resource dir doesn't exist
  • embedded_resources are in a separate crate
    • reduces parse times for the desktop crate (dir is included as byte string, about 10 sec extra build time)
    • makes it very clear what the build script is used for
    • no build script in desktop
  • allows specifying GRAPHITE_RESOURCES when build without the embedded_resources feature
  • preparation for watch command for desktop
  • Cleans resource handler code (moved to cef internal module etc.)

@Keavon
Copy link
Member

Keavon commented Aug 26, 2025

I'm unfamiliar with what kinds of resources these are, could you give some examples please?

@timon-schelling
Copy link
Member Author

I'm unfamiliar with what kinds of resources these are, could you give some examples please?

The frontend files, frontend/dist.


#[derive(Clone)]
pub(crate) struct Resource {
pub(crate) data: Vec<u8>,
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to make this a Cow<'static, [u8]> ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The results of a fs::read cannot be 'static, see usage of this struct.

Copy link
Member Author

Choose a reason for hiding this comment

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

if let Ok(data) = std::fs::read(file_path) {
    return Some(Resource { data, mimetype });
}

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but include dir should give you static references, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but this pr also adds the ability to create resources from disk. Could be behind feature flag for now, but would like to have override functionality in the future.

@github-actions github-actions bot requested a deployment to graphite-dev (Preview) August 28, 2025 16:34 Abandoned
@timon-schelling timon-schelling merged commit 1d4d102 into master Aug 28, 2025
4 checks passed
@timon-schelling timon-schelling deleted the desktop-optional-embedded-resources branch August 28, 2025 17:18
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.

4 participants