bennymeg / nx-electron

Electron schematics for nrwl nx platform
Apache License 2.0
309 stars 82 forks source link

Update to Nx Version 16 #217

Closed jasonbdt closed 9 months ago

jasonbdt commented 1 year ago

Is your feature request related to a problem? Please describe. No, there is no current problem related with.

Describe the solution you'd like We already were very excited to develop our project with Nx Workspaces at v15. I think it would be good if we start to migrate as soon as possible to the latest release.

Describe alternatives you've considered I think there's is no alternative

Additional context

Tasks

I've already forked the repository to make some changes. I'll try to help with this as much as possible✌️😉

jasonbdt commented 1 year ago

As Nx rescopes there official packages from @nrwl/* to @nx/* we need to wait, until the npm package jest-preset-angular updates to v13.0.2 (see: https://github.com/thymikee/jest-preset-angular/pull/2061), as with v13.0.1 the highest version of Angular we're able to use is v15.2.9.

@nx/angular versions history starts with v16.0.0

jasonbdt commented 1 year ago

As said above I've forked the repository and already made some changes. My first goal is to stop using methods that are already deprecated and will be removed in further versions of @nx.

@bennymeg Are you available if I need help at fixing the unit tests of nx-electron? I might have some questions with this.

bennymeg commented 1 year ago

Sure, I think the best way to address the unit tests issue is to migrate the latest tests from @nx/node and then add the specific tests for electron.

AshkanYarmoradi commented 1 year ago

Hi @bennymeg, @jasonbdt , Any update for this migration?

AshkanYarmoradi commented 1 year ago

Please look at this PR; It's helpful #219

abf7d commented 1 year ago

I wanted to check in on this as well. I appreciate the work you are doing. Is there any recent update?

bennymeg commented 1 year ago

I haven't got a chance to verify the PR. I will publish it tonight as an alpha so you could test it.

mtrefzer commented 1 year ago

Unable to use the recently released version 16.0.0-alpha I' getting the following error during npm i

npm WARN Could not resolve dependency:
npm WARN peer @nx/workspace@"^15.9.2" from nx-electron@16.0.0-alpha.0
npm WARN node_modules/nx-electron
npm WARN   dev nx-electron@"^16.0.0-alpha.0" from the root project
npm ERR! code ETARGET
npm ERR! notarget No matching version found for @nx/workspace@^15.9.2.
npm ERR! notarget In most cases you or one of your dependencies are requesting
npm ERR! notarget a package version that doesn't exist.
SouchetJulie commented 1 year ago

I suspect it's a simple oversight, but the 16.0.0-alpha version is tagged as latest on npm instead of alpha. You might want to change that so users download a stable version by default 😉

bennymeg commented 1 year ago

@SouchetJulie Tnx, that was a mistake, I'll fix it @mtrefzer, I think I know what the issue is, I'll fix it too tonight.

mtrefzer commented 1 year ago

@bennymeg tested version 16.0.0-alpha.1 and everything works perfectly fine. Thanks ;)

mpellerin42 commented 1 year ago

@bennymeg I just tested version 16.0.0-alpha.1 as well and everything worked as expected. Thanks !

mtrefzer commented 1 year ago

@bennymeg I just noticed that nx g nx-electron:app is not working because of the missing implemetation in nx-electron/src/generators/init/generator.ts.

function setDefaultCollection(tree: Tree, arg1: string) {
  throw new Error('Function not implemented.');
}
abf7d commented 1 year ago

@bennymeg, I am getting Function not implemented as well. when I try using nx g nx-electron:app

WillPoulson commented 11 months ago

I've just raised a PR which resolves the error on app generation here #232.

From some local testing with the linked npm package this seems to have solved it. 👍

As a temporary workaround for anyone else experiencing this, It's not ideal but I was able to generate a project in version 15, then upgrade to 16 without any issues 🤞

alkrobinson commented 11 months ago

Serving works fine for the nx-electron@16.0.0-beta.0. But for some reason running the build directly produces the error below. Any ideas?

5b1293d6-e950-45f3-8508-42f538eb4b98 ac167e32-a280-4036-b8a7-4e8576d5f231

WillPoulson commented 11 months ago

@alkrobinson

It looks like something potentially introduced when develop was merged into the v16 feature branch. A sneeky little @nrwl import came back in.

As I already had a PR open I've just tacked that change onto the end #233 (Bad practice but it was a one liner!)

Not 100% sure if that'll resolve your issue here as I can't replicate myself, but it would add up. What NX version are you attempting to use 16-beta with?

bennymeg commented 11 months ago

@WillPoulson Thanks! released a new fixed version.

abf7d commented 11 months ago

Hi @WillPoulson, I was able to generate a new electron app. There is an error I get in the console when I try to run it. It looks like it is running though. Here is the error I get:

Debugger listening on ws://127.0.0.1:5858/3b35a75e-d17a-499e-9ea7-bed2bdb2ecdf
For help, see: https://nodejs.org/en/docs/inspector

ERROR in apps/electron-shell/src/app/events/update.events.ts:38:27
TS2345: Argument of type '{ type: string; buttons: string[]; title: string; message: string; detail: 
string; }' is not assignable to parameter of type 'MessageBoxOptions'.
  Types of property 'type' are incompatible.
    Type 'string' is not assignable to type '"error" | "info" | "none" | "question" | "warning"'.    
    36 |     };
    37 |
  > 38 |     dialog.showMessageBox(dialogOpts).then(returnValue => {
       |                           ^^^^^^^^^^
    39 |         if (returnValue.response === 0) autoUpdater.quitAndInstall();
    40 |     });
    41 | });

Do you have any ideas on what this can be? Is this a typescript issue? Thanks

bennymeg commented 11 months ago

not sure what happened, just delete 'update.events.ts' if you are not using it.

alkrobinson commented 11 months ago

@WillPoulson I am on Nx version 16.5.3. The build issue is now resolved in 16.0.0-beta.1 but now I am getting errors in the make/package commands. It says it cannot find the built packages as soon as I run it. It doesn't seem to be packaging though. Any ideas? 0abaaea0-1cea-4bbf-a7d9-e9210d430342

WillPoulson commented 11 months ago

@alkrobinson Try running the :make command with --sourcePath=dist/apps. I encountered this same issue yesterday.

Another thing that slipped in from develop that isn't quite right with nx 16 I think.

I'll raise another PR to switch it back to use /apps by default and also attempt to resolve the issues that @abf7d ran into 👍

abf7d commented 11 months ago

Would we be able to get a publish? I apologize for the multiple requests.

WillPoulson commented 11 months ago

@abf7d Whilst waiting for the new version to be released, if you've got an existing generated project you can make the following changes to get the update.events.ts file to compile without any errors.

On line 1 add the import MessageBoxOptions. (import { app, autoUpdater, dialog, MessageBoxOptions } from 'electron';)

And on line 31 set the type of the dialogOpts variable to MessageBoxOptions. (const dialogOpts: MessageBoxOptions = {)

My PR modified the template files with these changes, so after it's released it would only fix newly generated projects anyway.

Hope this is helpful to you, let me know if you've got any questions

abf7d commented 11 months ago

@WillPoulson, that worked! Thanks so much.

dmatora commented 10 months ago

tried yarn add --dev github:WillPoulson/nx-electron#feature/resolve-nx-upgrade-issues still getting NX Unable to resolve nx-electron:app on nx g nx-electron:app desktop-electron after that

WillPoulson commented 10 months ago

@dmatora I'd avoid using my forked version- there's additional fixes that came in branches afterwards. There's currently a 16.0.0-beta.1 version on npm which should be working.

This will need to be ran in an NX 16 project to work.

dmatora commented 10 months ago

@WillPoulson I was looking for this in github branches/tags, but couldn't find it. Any clue how/why it's on npmjs only?

dmatora commented 10 months ago

Tried creating NX 16 project and using 16.0.0-beta.1 in it. It throws Cannot find module '@angular-devkit/core'

After doing yarn add @angular-devkit/core It started throwing Cannot find module '@angular-devkit/schematics/tools'

After doing yarn add @angular-devkit/schematics creating electron project throws NX Collection "@nx/workspace" cannot be resolved.

dmatora commented 10 months ago

Not sure what's changed, but today I tried again and suddenly it worked (well, almost)

  1. still requires yarn add @nx/jest (same as nx@15 requires yarn add @nrwl/jest)

  2. same ad nx@15 warns > NX Tree modified after commit to disk. this one gives > NX Tree changed after commit to disk.

  3. there is mentioned above issue with update.events.ts (I think I have same issue on nx@15) Fixed by replacing type: 'info' with type: 'info' as const)

  4. project is failing to package Fixed by adding targets.package.options.sourcePath = "dist/apps" in project.json

  5. built app is named weirdly @testsource and crashes instantly when I try to run it with Thread 0 last branch register state not available.

dmatora commented 9 months ago

Finally managed to get NX 16 working. Created boilerplate in case anyone needs it https://github.com/dmatora/nx-electron-boilerplate

bennymeg commented 9 months ago

@dmatora why you added dependency for @nrwl/workspace: 15.9.2?

dmatora commented 9 months ago

without it, making electron package was crashing

dmatora commented 9 months ago

however I just tried to reproduce the issue and couldn't I've removed that commit from main and backed it up into separate branch

bennymeg commented 9 months ago

Released