Skip to content

Ongoing development (part 2) - #35

Open
vegeta897 wants to merge 23 commits into
iznaut:mainfrom
vegeta897:dev
Open

vegeta897 wants to merge 23 commits into
iznaut:mainfrom
vegeta897:dev

Conversation

@vegeta897

Copy link
Copy Markdown
Contributor

Trying to make the huge files more wieldy since their size is actively slowing down my other work.

I want to make the tray more responsive to changes without having to rebuild it all the time.

@vegeta897

vegeta897 commented Aug 17, 2026 •

Copy link
Copy Markdown
Contributor Author

Okay, real quick because it's 5:49AM

I updated Electron one major version (40). There are no breaking changes that affect the project, but it allows MenuItem labels to be dynamically updated (without calling Menu.setContextMenu()).

Yes, we now have a tray menu that is built once and then dynamically updated! I added an EventEmitter to the projects module that emits activeProjectChanged, which the tray listens for and updates all the relevant menu item labels, visibility, and enabled status, as needed. Not sure your thoughts on using events when we could call an exported method from tray.js. Now I say it out loud I'm not sure I want to use events.

Still TODO is updating the settings menu, initial version check, and also inserting new projects to the projects submenu.

@vegeta897

Copy link
Copy Markdown
Contributor Author

Rewrote the settings submenu and added the initial version check.

Found out the crash reporting checkbox would uncheck itself even if you said to keep it on in the prompt (visual bug, the setting itself still saved properly). Makes sense, electron handles the checking and unchecking, but I had to add a line to re-check it if the user says keep it on.

@vegeta897

vegeta897 commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor Author

New projects are now added to the projects submenu by checking if the activeIndex exceeds the number of projects already there.

The complexity of updating the projects submenu is starting to make me reconsider full tray menu rebuilds. Unfortunately Electron doesn't let you rebuild only the submenu. You can't even remove items from a menu, only insert them (this could be gotten around by hiding ones we want to "delete", but this makes things more complex).

This is ending up more fragile and harder to maintain than seems worth it. I might switch back to full menu rebuilds, at least for when the active project changes, and maybe deployment stuff. Simpler things like update checks could stay as menu updates.

Also somewhat related, while app/index.js is now a manageable size, app/tray.js is now cumbersome to navigate at over 400 lines. I will consider creating a app/tray directory and splitting sections of the tray into different files.

@vegeta897

vegeta897 commented Aug 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Okay, we're back to tray rebuilds and I'm much happier about it. Also the tray rebuild can be called from other files such as projects.js so we don't need that event emitter anymore. Oh, and the ability to just quickly update individual menu items is still there, and used for the "update available" item and the crash reporting re-checking when the user decides to keep it on. Best of both worlds!

Also, I added ESLint as a pre-typescript measure. I was reeeeally missing my IDE not yelling at me for unused vars, or vars I was using that weren't defined. Way too prone to making mistakes without this. Enabling it already let me catch a bunch of bugs and cleanup I missed. I recommend installing the eslint extension.

The config file for ESLint is default except for 2 things I added:

Adding logger to the globals

globals: {
    logger: "readonly",
}

Allowing sendForm as an "unused" function defined in formHandler.js

rules: {
    "no-unused-vars": [
        "error",
        {
            varsIgnorePattern: "sendForm",
        },
    ],
}

@vegeta897

Copy link
Copy Markdown
Contributor Author

Back at it, with some good progress.

SFTP deployment now re-prompts for the password. To support this, I rewrote some of the deploy form handling and deploy function code to introduce the concept of "ephemeral" deploy properties, which don't get saved but are merged with the saved properties to execute the deployment. The password prompt gets to re-use the deploy form handling logic, just with a stripped down form.

Also, the provider name supplied to SFTP deployment setup wasn't being used, so I re-implemented that. And to allow the password prompt form to show that custom name, I added a data = {} argument to the form renderer so that a custom title could be passed in.

Less exciting is an attempt to fix errors caused by fs.rmSync randomly failing. Node helpfully provides a maxRetries parameter, but the timing between retries is bugged in the sync version, so I changed it to the promise-based one. It seems to be working okay now but like I said it was kind of random. From what I gathered, Windows can just be finnicky about file access sometimes.

@vegeta897

vegeta897 commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Initial work on #39

image image

Each deploy method now has a verifyOnly argument which just verifies the provided credentials result in a successful connection. The result is displayed as success or an error message below the button (wip styling & wording)

I also got around to testing Neocities, and added/updated some related TODOs.

Also at some point during testing, a .vite folder ended up in my _site?

@vegeta897

Copy link
Copy Markdown
Contributor Author

I put this in a comment, but we should reconsider using alwaysOnTop: true in our browser windows. It can obscure Electron-native dialogs and runtime errors.

@vegeta897

vegeta897 commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor Author

I was, once again, getting tired of every file that wasn't src/app/index.js being treated as a renderer file and failing to trigger reloads in electronmon. I looked for more current solutions for live-reloading electron apps but couldn't find anything that seemed really good (mostly based on vibes, but also had trouble getting some of them working). I was considering electron forge but the current version adds a ridiculous amount of npm vulnerability warnings, and it seems overkill anyway. I dunno, nothing I found felt right, which was frustrating for something that feels so essential.

I did find out after digging through its source that electronmon is failing to recognize main process files because it relies on a package that checks for require instead of import. That's what it seems like anyway. I might just fork it and tweak it to our needs.

I should also get around to trying to optimize the app startup time. It would ease a lot of this friction if it could be significantly reduced. Reminder to self: create an empty electron starter project and see how fast that reloads, to see what the lower limit could be.

@vegeta897

Copy link
Copy Markdown
Contributor Author

I created an empty electron project to see how fast things could be, and wow yeah it was pretty much instant startup/reload time.

With that goal in mind, I referred back to the optimization tips in the Electron docs, and one part mentions avoiding thread-blocking file i/o (eg. fs.writeFileSync) and using the async fs functions. So I replaced almost every instance of sync with awaited promises. In some cases, I rewrote some loops that were doing file i/o with Promise.all which allows them to run in parallel. But even without that, keeping these off the main thread is helpful in general.

While doing that, I took the opportunity to replace a lot of lodash calls with built-in methods and patterns. I'd love to eliminate lodash entirely (I don't think lodash is slowing things down, but so much of lodash is unnecessary) and even the handy functions like merge or omit aren't really needed with some rewriting (I don't think we even want lodash's deep merging in the places we're currently using it, as it merges property values instead of letting us override them).

None of this is speeding up the start time yet. I believe the main culprit here is how many modules we're importing and initializing immediately. I'll have to hunt these down.

@vegeta897
vegeta897 marked this pull request as ready for review September 19, 2026 02:13
@vegeta897

vegeta897 commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor Author

Yeeeaaah, bimbo now starts up like 500% faster: from over 4 seconds to ~0.7s on my machine.

Now, this wasn't magic. Time until the first site build is roughly the same, maybe a bit quicker. But getting that app icon appearing as quickly as possible, as well as the welcome screen for new users, is a great boost for UX (not to mention being much nicer in dev mode with app reloads). There is no impact to site re-build time, so the rest of the app isn't being slowed down during use.

This all came from deferring imports, which means getting rid of the imports of node modules at the top of the file and replacing them with awaited import() calls right when they're needed. Every module that was being imported before being used immediately added startup time. I profiled a bunch of the modules we were using to check their impact, but mostly just deferred everything that made sense to and could be done easily.

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