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
Undocumented breaking change for Vite plugin in v7.3.0 #3506
Comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Do you mind pointing to the change that fixed the issue? There seems to be a lot going on there. Here’s my files: https://github.com/will-stone/browserosaurus |
Chiming in here... I've been using configs based on modified 7.2.0 templates (vite + electron + svelte! 😉 ). Upgrading to 7.3.0 causes my existing configs to ... just not produce any output! (.vite/build/... empty: no main.js found). I think I might be able to get things to work if I rebuild the changes in my templates on top of the new ones, but that seems involved for a minor release. Note: I don't think it's the vite@5 update that's causing my issue, as my package.json already specified |
This comment was marked as duplicate.
This comment was marked as duplicate.
The only insight I didn't mention is that the main PR description ( #3468 ) admits
So, I think the expectation here is we recreate our build configs on the new base templates? |
Can you provide a migration guide, to fix this issue? |
+1 ! |
Migrate to
|
I'm a little concerned that this migration guide is implying the de-facto official path uses Ideally any migration utility should be contributed and maintained under |
|
Well, migrating any code from It would be nice if you would explain what was moved, and if possible, please describe the reasons that these were moved (rather than claiming in general "to make it easy to fix some errors" or "very fast release pace of Vite"). Adding another dependency which makes the system an interaction of the versions of forge, vite, the client template, and your migration base doesn't seem like a good idea for anyone involved! BTW, you've also claimed that these changes enable esm support but as others have pointed out, how these changes enable esm support is not clear. Do you have an example to share with them? |
Personally, the way this "migration" is being handled makes me want to run for the hills. I don't think it is acceptable that an upgrade from 7.2 to 7.3 breaks my code, without any warning. The reason I am using something like forge in the first place is because I don't want to look below the abstraction that covers all the messy details. |
@sparecycles in reviewing several of the related gh issues, it seems as if you and I have come to a similar conclusion and have tried similar “workarounds” to get things working for esm/type:module. Specifically to your quote above, I have been coming to a similar conclusion. It’s a bit of a shame because I have been building and fine tuning our vite-ts-forge config the last year or so and there is a lot in there that I hate to lose / re-implement, but at the same time I think that is part of the issue for me for troubleshooting recent changes, because I have a lot of configurations items that could be throwing things off…. And some are likely no longer necessary. TLDR: after spending the last couple days trying to “hack” this to work with our current build/ci flows, I think I am going to start from scratch, get esm / type module working (without having to change the format of our import statements etc in our codebase) and then rebuild some of our customization on top of it after the dust settles. are you thinking similarly? Do you have any public working MVPs is could peak at? Thanks! |
@GitMurf unfortunately I'm sort-of developing an in-house project... I'm the sole contributor but I haven't gone through the steps to open-source it and there are a few more features I'd like to build before making it public. (Perhaps I'm being too careful.) This has gone on long enough that I should probably make a small svelte + vite + electron demo project (with both 7.2.0 and 7.30) to share. |
Okay! only took about 30 minutes to strip the project down to the vite/svelte/electron/tailwind integration. If you're not a mac user, you may need to add the windows/linux plugins back in (I tend to remove stuff from templates unless I know what they do). a I'll try to do the |
If you manually merge in the new templates, it does work. https://github.com/sparecycles/vite-svelte-tailwind-electron/compare/forge-7.2.0..forge-7.3.0 It's a little curious though, the three forge configs interact with the "vite.base.config" file differently:
Also the renderer config does not Now it was actually pretty easy at this point to remove my old hack and switch to an esmodule build target! |
@sparecycles thanks so much for these updates and mvp! On your first one I was going to ask you where / how you were changing from vite cjs to es. But it looks like now in hour update you have that working? Also, your comments about windows (we build our app for windows and macs) is it really just the makers in your forge config that you stripped out windows stuff? If so that is fine for me because we have some complex package and makers customizations anyways :) |
@GitMurf it's mostly the makers, I've also attempted to get windows builds working with a non-developer coworker and it hangs (there's issues with native compilation: python 3.12 removed their deprecated distutils package so only node-gyp 10+ supports python 3.12+ and updating node-gyp theoretically needs a one-line change to be supported in electron-rebuild... at least, just patching that change in on mac works, but their machine failed to build and took a long time)... So I guess the makers aren't even needed, I've never gone the extra step of packaging the app into a diskimage or whatever the darwin-specific maker does. But FWIW, I can't yet support windows issues because I can't test. (started working on spinning up a windows VM this weekend, but things never go smooth!) I think I can make an alternate config.base.ts that could provide methods to mix-in these new config requirements with less code needing to change (wrap the vite config.ts file's default exports with a function that mixes in the functionality). Now that I've done the work it's not that bad, but it's annoying for sure to see this framework-type code moved into client-land. |
@GitMurf Okay, I did the thing. forge-7.3.0-rewrite-esm-support:./vite.base.config.ts should be a shim that helps people migrate. Copy-paste that file into the project, and then try taking whatever your existing configs are and wrap them with the appropriate the only non-trivial (non export default extendRendererConfig({
plugins: [svelte({})],
}); because svelte! I've only tested this with Thanks for the nudge, I feel like I can finally update my actual project now! @Ben-Avrahami @boldt this may be easier to apply for you. EDIT: oof, it appears that I didn't test preload. It seems to be imported by require in electron and therefore needs to be a |
You only see that there is an increase in template code for
BTW, if you feel that there is too much |
Maybe extending the data in e.g., new VitePlugin({
baseless: true, // basic configuration will be provided by application instead
build: [
{
entry: "electron/main.ts",
config: "vite.main.config.ts",
},
{
entry: "electron/preload.ts",
config: "vite.preload.config.ts",
},
],
renderer: [
{
name: "main_window",
config: "vite.renderer.config.ts",
},
],
}) Forge could make that flag the default in a major version release: that's when people expect there to be breaking changes, meanwhile anyone not able to extend their templates is blocked from updating their forge minor version for any other reason! Maybe the electron/forge maintainers can join this conversation? Are @erickzhao @VerteDinde slackers? (oh yes, yes they are.) |
it is possible to use |
I don't even know what you mean by "template code" (I guess I will have to find out now), so I don't care if there is much of it or not. All I care about is that I can use TypeScript to write a desktop app, and hot reload is nice (although that feature seems limited, as server code doesn't hot reload, it seems). That's about the level of abstraction I want to think about here. Unfortunately, that approach does not seem viable (it seemed to be, until 7.2 -> 7.3 came along), so this "talent" needs to dig deeper into the mess now. |
@caoxiemeihao I am fairly well versed in forge and vite but there is so much conversation here (and in other gh issues) about what needs to be patched where and what forge version and running migration scripts etc etc that I am confused and a bit lost. Is it possible that you publish a minimal working template repo that has everything we need with forge 7.4 to work with forge-vite-typescript and ESM (type: module)? Basically what the vite typescript template should do (but it sounds like there are reasons why the template can't work or something?). I just think we need some "official" docs / direction that will also be maintained / supported by forge long term otherwise it will turn into a maintenance nightmare. Thanks! |
I know the changes I need to get my example repo into a fully working state with forget-7.4.0 (support |
Hey folks, sorry for our delay - we spoke about this in a maintainer meeting today. We're deciding on a path forward, but wanted to briefly update the issue letting you know that there are currently eyes on this and we should be reporting back with fixes shortly. |
Ok I think I finally understand the issue. Am I correct that there were changes to the forge vite plugin that required changes to the vite configs? And those changes are part of the forge vite template? But the problem is on existing projects, the templates do not update as it is just starter template code. So for a brand new project, it would work, but for existing projects, you have to manually copy changes that were made to the vite configs that are available in the template? |
@GitMurf That is my understanding 👍 As @VerteDinde mentioned above we're looking into options here, the issue is the breaking change is bidirectional. i.e. straight up reverting won't help because then new apps will be broken in the opposite way to old apps. Right now you should pin to a version of forge that works for your app, and we'll figure out how to make a way forward to higher forge versions. |
Thank you @MarshallOfSound for your quick response... I feel better that I at least understand the problem now :) if/when you need testers, let me know. |
I'm not sure about that. If the missing configuration is left in the template and restored to the plugin, the duplicated configuration should mostly merge back together. 🤞 ❔ |
That led to this problem: electron/forge#3506 and this problem: electron/forge#3208 Fixing those led to changes in logger build, which required updates in downstream code
Pre-flight checklist
Electron Forge version
7.3.0
Electron version
N/A
Operating system
All
Last known working Electron Forge version
7.2.0
Expected behavior
Upgrading your Vite plugin app from Electron Forge v7.2.0 to v7.3.0 is seamless.
Actual behavior
Changes in the build system during the upgrade to Vite 5 have produced an API-incompatible change, requiring users to upgrade.
Steps to reproduce
N/A
Additional information
To fix this issue, see the
@electron-forge/template-vite
template config files: https://github.com/electron/forge/tree/main/packages/template/vite/tmplThe text was updated successfully, but these errors were encountered: