Skip to content

Conversation

@calcaide
Copy link
Contributor

@calcaide calcaide commented Mar 20, 2025

Description

Upgrade ember-electron from 4.2.2 to 6.0.0. In preparation for electron upgrade

As per changelog, it is safe to upgrade.

🎟️ Jira ticket

UPDATE: We change the goal from ember-electron v7 to v6. After reproducing some issues and debugging, ember-electron v7 does not support ember < 4.8 and we are using ember 4.12. So for pragmatism we decided to upgrade to 6. Once the ember upgrade is complete, we will review this dependency.

How to Test

@calcaide calcaide self-assigned this Mar 20, 2025
@vercel
Copy link

vercel bot commented Mar 20, 2025

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

Name Status Preview Comments Updated (UTC)
boundary-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 26, 2025 9:25pm
boundary-ui-desktop ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 26, 2025 9:25pm

@calcaide calcaide marked this pull request as ready for review March 20, 2025 18:37
@calcaide calcaide requested a review from a team as a code owner March 20, 2025 18:37
@calcaide calcaide marked this pull request as draft March 20, 2025 21:23
Base automatically changed from chore-upgrade-electron-forge to main March 21, 2025 18:27
hashicc
hashicc previously approved these changes Mar 24, 2025
Copy link
Collaborator

@hashicc hashicc left a comment

Choose a reason for hiding this comment

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

Looking at the ember-electron changelog I agree it looks safe. Also I think the other @electron-forge/* have already been merged here, so this PR is really just ember-electron.

@calcaide
Copy link
Contributor Author

@hashicc

Thaks for the feedback. That is correct, this PR initially was a branch from the electron-forge upgrade, now pointing to main, Since electron-forge changes have been merged.

Still working on a blocking issue, that's why marked as draft, will update the PR as soon I unblock 😉 🤞

@calcaide calcaide changed the title Upgrade ember-electron from 4.2.2 to 7.0.0 Upgrade ember-electron from 4.2.2 to 6.0.0 Mar 26, 2025
@calcaide calcaide marked this pull request as ready for review March 26, 2025 21:52
@calcaide calcaide requested review from a team and hashicc March 26, 2025 21:52
Copy link
Collaborator

@ZedLi ZedLi left a comment

Choose a reason for hiding this comment

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

After reproducing some issues and debugging

What were the issues? Considering we were already on 4.12, I'm curious what the problems were

@calcaide
Copy link
Contributor Author

@ZedLi

My apologie, rather than answering your comment I click "Edit" and answer within your own comment.

I corrected that and now a propper answer:

The test finishes but not the process that runs it. In local test, test process keeps running infinetely, until you manually stops it. In CI it times out. I didn't digg that much to indentify the cause of the issue tho.

My findings are:

  • We use a testem-electron file that loads the test runner from ember-electron. Here file
  • We use a test wrapper within electron-app/ that call helpers from test-support.
  • Initially I though the issue must be related to these 2 files mentioned above, checked ember-electron changes on those between v6 and v7 and found no differences, so I start thinking it can be related to 3rd party deps those processes uses such as electron-devtools-installer. I digg a bit on those but didn't find a good breadcrumb to follow.
  • I reproduced the same behaviour on Mac and Linux, so thinking its not OS/env related.
  • I also find out in v7 they dropped support for < ember 4.8. After this I thought best to keep the ember upgrade and other deps rolling, and attempt ember-electron upgrade after Ember + Electron upgrade.

Open to discuss this further if need.

Copy link
Collaborator

@DhariniJeeva DhariniJeeva left a comment

Choose a reason for hiding this comment

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

looks good! thanks for the upgrade!

@calcaide calcaide merged commit 4b1327d into main Mar 27, 2025
13 checks passed
@calcaide calcaide deleted the chore-upgrade-ember-electron branch March 27, 2025 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants