Skip to content

Fix electron type to reflect that it could be undefined #8019

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jtran
Copy link
Contributor

@jtran jtran commented Aug 8, 2025

The main change here is not making the type of window.electron lie. Instead of being IElectronAPI, this PR changes it to IElectronAPI | undefined. Everything else falls out of that.

  • Instead of checking if isDesktop() is truthy, check window.electron. It's natural type narrowing.
  • This adds a dependency on https://github.com/chaiNNer-org/node-path so that Windows-aware path functions are available in the web, and there's less error checking.
  • Rename fileSystemManagerprojectFsManager to make it clear that it's doing everything relative to the project directory. If you don't want this functionality, use fsManager, which is another instance where we don't set the project directory.
  • Some functions take an electron parameter but don't use it. This is because they access the filesystem some other way, like fsManager. But they only make sense in Electron. So by requiring an Electron argument, people cannot accidentally call it in the web. You can think of it as providing proof of Electron. If we ever implement another way to store files on the web, like cloud storage, we can remove this parameter.

Copy link

vercel bot commented Aug 8, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 9, 2025 11:15pm

Copy link

codspeed-hq bot commented Aug 8, 2025

CodSpeed Instrumentation Performance Report

Merging #8019 will not alter performance

Comparing jtran/electron-type (3bfbe86) with main (7235ab4)

Summary

✅ 89 untouched benchmarks

} from 'fs'
import type { Abortable } from 'events'
// @ts-ignore This lib doesn't have types.
import * as nodePath from '@chainner/node-path'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This lib doesn't have types. Should we find another one?

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.

1 participant