nxext / nx-extensions

Nx Extension for StencilJs, SvelteJS, SolidJS, Ionic and VueJs
MIT License
454 stars 99 forks source link

Capacitor: On 16.2 node modules get removed after sync step #962

Closed arendjantetteroo closed 1 year ago

arendjantetteroo commented 1 year ago

First off, thanks for the work that goes into these extensions!

Describe the bug I depend on the node_modules in my projectRoot to be left as is after the sync step. This way i can add the full directory as an artifact, so I can in a later ci job use the capacitor plugins without needing a nodejs environment to install them. This way i can just build my android app with an android docker environment that isn't cluttered with nodejs.

To Reproduce Since this commit on running a capacitor command, at the end the node_modules directory is removed from the project root. https://github.com/nxext/nx-extensions/commit/c2beee95bf8d8647d55fff0350cf915cab971ec5

https://github.com/nxext/nx-extensions/blob/65684f656a67fb0302c59c53f7dc1fc04b9f6d9e/packages/capacitor/src/executors/cap/executor.ts#L70

Steps to reproduce the behavior: Run app:sync for a capacitor project based on a nextjs project. (so i run with app:build, then app:export)

Expected behavior Having a flag to keep the node_modules or a way to copy them first into a dist directory, alongside the ios/android directories. So on running sync, i expect to have a directory somewhere with the ios and android directories alongside their needed node_modules containing the java/swift code and anything else needed so either android studio/xcode or tools like fastlane can package that up into a native app package without needing a nodejs environment.

If there is another/better way to do this, i'm open to suggestions.

DominikPieper commented 1 year ago

@arendjantetteroo I added a option to preserve the node_modules folder

arendjantetteroo commented 1 year ago

@DominikPieper i tried on 16.2.1 but i'm probably doing something wrong.

I've got this step in my app/project.json file:

"sync": {
      "executor": "@nxext/capacitor:cap",
      "options": {
        "cmd": "sync",
        "preserveProjectNodeModules": true
      },
      "configurations": {
        "ios": {
          "cmd": "sync ios"
        },
        "android": {
          "cmd": "sync android"
        }
      }
    },

and then run with "app:sync" and still get this output:

[info] Sync finished in 0.249s

Removing node_modules from project root...

Should that option be somewhere else in my config?

arendjantetteroo commented 1 year ago

@DominikPieper

Looking at the generated js code in my node_modules i see this:

if ((0, fs_1.existsSync)(nodeModulesPath) && preserveProjectNodeModules) {
            try {
                devkit_1.logger.info(`\n\nRemoving node_modules from project root...`);
                (0, fs_1.rmSync)(nodeModulesPath, { recursive: true, force: true });
            }
            catch (err) {
                devkit_1.logger.error(`\n\nFailed to remove node_modules from project root.`);
            }
        }

So if i use preserveNodeModules = false in my project.json, i get the behaviour i expect. But that naming seems inverted? I would expect preserve = true to keep my node_modules.

DominikPieper commented 1 year ago

@arendjantetteroo ah damn. It was to late yesterday ^^'

arendjantetteroo commented 1 year ago

@DominikPieper it happens, i'm happy you already got back to this issue so quickly in the first place. :)

DominikPieper commented 1 year ago

@arendjantetteroo merged it. I'll do a hotfix release soon. Don't mind the failed run. That's the ionic-react linter tests. I have to fix the linter setup

arendjantetteroo commented 1 year ago

@DominikPieper thanks, greatly appreciated!