search

simondotm / nx-firebase

Firebase plugin for Nx Monorepos
https://www.npmjs.com/package/@simondotm/nx-firebase
MIT License
175 stars 31 forks source link

Nx 13.0.1 release breaks nx-firebase use of createProjectGraph() #44

Closed agalper closed 1 year ago

agalper commented 2 years ago

In recent release of Nx 13.0.1, createProjectGraph() is no longer a public API.

As a result the build executor fails for Firebase functions:

nx run functions:build project_graph_1.createProjectGraph is not a function

Looks like createProjectGraphAsync() is still available. There may be more changes needed as well.

Any advice on working around this with Nx 13.0.1 is much appreciated.

jimlloyd commented 2 years ago

I too have hit this issue. I tried pinning all of the nx components to the 13.0.0 release but the project_graph_1.createProjectGraph is not a function was not fixed. Pinning to the 12.10.0 works.

RandomDude123 commented 2 years ago

was able to make it run locally after updating to 13.1.2.

I don't manage to run the e2e-tests locally and don't understand the logic behind the nonBuildableDeps checks and therefor can't fix them. A PR seems not really useful at this point...

edriang commented 2 years ago

I'm having issues to, with version 13.1.4

simondotm commented 2 years ago

Hi folks, I'm very sorry for the radio silence recently I had some personal issues to deal with. I'll be able to look into this over the next week.

tomschreck commented 2 years ago

I tried @RandomDude123 recommendation in /node_modules/@simondotm/nx-firebase/src/executors/build/build.js. Running NX's build command on project seems to get stuck at 'Copying asset files'.

UPDATE 1 On a whim, I deleted references to entries in "assets" config section of build target in the project.json file for the NX project created by @simondotm/nx-firebase. After making "assets": [], then the build completed.

UPDATE 2 I had to update the functions.predeploy array inside firebase..json file. When the project gets created, the predeploy is configured to call npx nx build... as well as npx nx lint.... I had to remove the build predeploy because terminal is getting hung up with a message of 'Done compiling Typescript files for project ...'. This is the same hang up I get when calling build by itself. So, I'm uncertain if this hangup is associated with the modifications I made in /node_modules/@simondotm/nx-firebase/src/executors/build/build.js in order to get @simondotm/nx-firebase to work with NX version 13.x.

If I run build by itself and manually break the hangup then manually call deploy, I can succesfully deploy the simple HelloWorld function to firebase's cloud functions. This is OK for now. Hopefully these things will be cleared up soon.

Looking forward to a fix because getting Firebase functions to fit within NX monorepo would be awesome.

BraunreutherA commented 2 years ago

Hey there,

I've created and PR: #45 - following @RandomDude123 work it was only missing to tell the createProjectGraphAsync to use the old version 4.0. This is only a fix for now and the project should get refactored to NX V13.

@simondotm I can put some effort into that an help if you already have a plan. Still I hope you can use this PR so we can go on using the plugin properly :)

miikebar commented 2 years ago

I've created a patch based on @BraunreutherA's PR:

@simondotm+nx-firebase+0.3.3.patch

diff --git a/node_modules/@simondotm/nx-firebase/src/executors/build/build.js b/node_modules/@simondotm/nx-firebase/src/executors/build/build.js
index ba14795..16aa79f 100644
--- a/node_modules/@simondotm/nx-firebase/src/executors/build/build.js
+++ b/node_modules/@simondotm/nx-firebase/src/executors/build/build.js
@@ -42,7 +42,7 @@ function runExecutor(options, context) {
         debugLog("Running Executor for Firebase Build for project '" + context.projectName + "'");
         debugLog('options=', options);
         // get the project graph; returns an object containing all nodes in the workspace, files, and dependencies
-        const projGraph = project_graph_1.createProjectGraph();
+        const projGraph = yield project_graph_1.createProjectGraphAsync('4.0');
         // nx firebase functions are essentially @nrwl/node:package libraries, but are added to the project
         // as applications as they are fundamentally the deployable "application" output of a build pipeline.
         // Due to this, we can import standard node libraries as dependencies from within the workspace
diff --git a/node_modules/@simondotm/nx-firebase/src/executors/build/node/package/package.js b/node_modules/@simondotm/nx-firebase/src/executors/build/node/package/package.js
index a3218e7..835ecb5 100644
--- a/node_modules/@simondotm/nx-firebase/src/executors/build/node/package/package.js
+++ b/node_modules/@simondotm/nx-firebase/src/executors/build/node/package/package.js
@@ -11,7 +11,7 @@ const normalize_options_1 = require("./utils/normalize-options");
 const cli_1 = require("./utils/cli");
 function packageExecutor(options, context) {
     return tslib_1.__awaiter(this, void 0, void 0, function* () {
-        const projGraph = project_graph_1.createProjectGraph();
+        const projGraph = yield project_graph_1.createProjectGraphAsync('4.0');
         const libRoot = context.workspace.projects[context.projectName].root;
         const normalizedOptions = normalize_options_1.default(options, context, libRoot);
         const { target, dependencies } = buildable_libs_utils_1.calculateProjectDependencies(projGraph, context.root, context.projectName, context.targetName, context.configurationName);

You can use it with with patch-package until the issue is resolved

shannonjohnstone commented 2 years ago

Just wanted to check in and see if we are any closer to getting #45 or another solution merged

tomschreck commented 2 years ago

checking in as well to see about getting #45 merged.

anandsathe67 commented 2 years ago

Using the latest (as of this moment) version of nx (13.4.0), the following patch works. Had to tweak a few more things in addition to @MrPumpking 's changes

diff --git a/node_modules/@simondotm/nx-firebase/src/executors/build/build.js b/node_modules/@simondotm/nx-firebase/src/executors/build/build.js index ba14795..5a62360 100644 @simondotm+nx-firebase+0.3.3.patch.txt

--- a/node_modules/@simondotm/nx-firebase/src/executors/build/build.js +++ b/node_modules/@simondotm/nx-firebase/src/executors/build/build.js @@ -42,7 +42,8 @@ function runExecutor(options, context) { debugLog("Running Executor for Firebase Build for project '" + context.projectName + "'"); debugLog('options=', options); // get the project graph; returns an object containing all nodes in the workspace, files, and dependencies

enchorb commented 2 years ago

Can confirm the original patch by @MrPumpking still gave an error (fileutils_1.readJsonFile is not a function) however @anandsathe67 's additions seem to have resolved that - functions deployed successfully!

johngrimsey commented 2 years ago

This saved my ass thanks @MrPumpking and @anandsathe67 !

jesusdpp96 commented 2 years ago

@anandsathe67's patch works fine. Now, the problem is when you installs new package ,changes are lossing (at less with 'yarn add' happened this)

johngrimsey commented 2 years ago

@jesusdpp96 Use https://www.npmjs.com/package/patch-package

amkoehler commented 2 years ago

Thank you, @MrPumpking @anandsathe67 and @BraunreutherA for your work on this! With nx 13.4 we're using the patch with patch-package and a postinstall script. It's working well but I suggest anyone using the patch to keep an eye on #45 to be merged so that the patch is no longer needed.

ProductOfAmerica commented 2 years ago

Hi folks, I'm very sorry for the radio silence recently I had some personal issues to deal with. I'll be able to look into this over the next week.

I donate if you fix <3

seanaguinaga commented 2 years ago

How do apply a patch from a text file?

wstidolph commented 2 years ago

@anandsathe67 could you repost your patch in code/text rather than rendered markdown so I could just save it as a diff patch file? (Thanks!)

victorouse commented 2 years ago

@anandsathe67 could you repost your patch in code/text rather than rendered markdown so I could just save it as a diff patch file? (Thanks!)

@simondotm+nx-firebase+0.3.3.patch

diff --git a/node_modules/@simondotm/nx-firebase/src/executors/build/build.js b/node_modules/@simondotm/nx-firebase/src/executors/build/build.js
index ba14795..e23fe0e 100644
--- a/node_modules/@simondotm/nx-firebase/src/executors/build/build.js
+++ b/node_modules/@simondotm/nx-firebase/src/executors/build/build.js
@@ -15,7 +15,6 @@ const normalize_options_1 = require("./node/package/utils/normalize-options");
 const cli_1 = require("./node/package/utils/cli");
 const workspace_1 = require("@nrwl/workspace");
 const fs_extra_1 = require("fs-extra");
-const fileutils_1 = require("@nrwl/workspace/src/utilities/fileutils");
 const ENABLE_DEBUG = false;
 function debugLog(...args) {
     if (ENABLE_DEBUG) {
@@ -42,7 +41,7 @@ function runExecutor(options, context) {
         debugLog("Running Executor for Firebase Build for project '" + context.projectName + "'");
         debugLog('options=', options);
         // get the project graph; returns an object containing all nodes in the workspace, files, and dependencies
-        const projGraph = project_graph_1.createProjectGraph();
+        const projGraph = yield project_graph_1.createProjectGraphAsync('4.0');
         // nx firebase functions are essentially @nrwl/node:package libraries, but are added to the project
         // as applications as they are fundamentally the deployable "application" output of a build pipeline.
         // Due to this, we can import standard node libraries as dependencies from within the workspace
@@ -144,9 +143,9 @@ function runExecutor(options, context) {
         const incompatibleNestedDeps = [];
         // rewrite references to library packages in the functions package.json
         // to be local package references to the copies we made
-        const functionsPackageFile = `${options.outputPath}/package.json`;
+        const functionsPackageFile = `${options.outputPath}/package.json`
+        const functionsPackageJson = devkit_1.readJsonFile(functionsPackageFile);
         debugLog("- functions PackageFile=" + functionsPackageFile);
-        const functionsPackageJson = workspace_1.readJsonFile(functionsPackageFile);
         const functionsPackageDeps = functionsPackageJson.dependencies;
         if (functionsPackageDeps) {
             debugLog("- Updating local dependencies for Firebase functions package.json");
@@ -164,7 +163,7 @@ function runExecutor(options, context) {
                 }
             }
         }
-        fileutils_1.writeJsonFile(functionsPackageFile, functionsPackageJson);
+        devkit_1.writeJsonFile(functionsPackageFile, functionsPackageJson);
         devkit_1.logger.log("- Updated firebase functions package.json");
         debugLog("functions package deps = ", JSON.stringify(functionsPackageDeps, null, 3));
         // Final dep check before we compile for:
diff --git a/node_modules/@simondotm/nx-firebase/src/executors/build/node/package/package.js b/node_modules/@simondotm/nx-firebase/src/executors/build/node/package/package.js
index a3218e7..43a4dc7 100644
--- a/node_modules/@simondotm/nx-firebase/src/executors/build/node/package/package.js
+++ b/node_modules/@simondotm/nx-firebase/src/executors/build/node/package/package.js
@@ -11,7 +11,7 @@ const normalize_options_1 = require("./utils/normalize-options");
 const cli_1 = require("./utils/cli");
 function packageExecutor(options, context) {
     return tslib_1.__awaiter(this, void 0, void 0, function* () {
-        const projGraph = project_graph_1.createProjectGraph();
+        const projGraph = yield project_graph_1.createProjectGraphAsync('4.0');
         const libRoot = context.workspace.projects[context.projectName].root;
         const normalizedOptions = normalize_options_1.default(options, context, libRoot);
         const { target, dependencies } = buildable_libs_utils_1.calculateProjectDependencies(projGraph, context.root, context.projectName, context.targetName, context.configurationName);
diff --git a/node_modules/@simondotm/nx-firebase/src/executors/build/node/package/utils/cli.js b/node_modules/@simondotm/nx-firebase/src/executors/build/node/package/utils/cli.js
index db9311d..4501336 100644
--- a/node_modules/@simondotm/nx-firebase/src/executors/build/node/package/utils/cli.js
+++ b/node_modules/@simondotm/nx-firebase/src/executors/build/node/package/utils/cli.js
@@ -1,11 +1,11 @@
 "use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
-const fileutils_1 = require("@nrwl/workspace/src/utilities/fileutils");
+const devkit_1 = require("@nrwl/devkit");
 const fs_extra_1 = require("fs-extra");
 function addCliWrapper(options, context) {
-    const packageJson = fileutils_1.readJsonFile(`${options.outputPath}/package.json`);
+    const packageJson = devkit_1.readJsonFile(`${options.outputPath}/package.json`);
     const binFile = `${options.outputPath}/index.bin.js`;
-    fileutils_1.writeToFile(binFile, `#!/usr/bin/env node
+    devkit_1.writeToFile(binFile, `#!/usr/bin/env node
 'use strict';
 require('${packageJson.main}');
 `);
@@ -13,7 +13,7 @@ require('${packageJson.main}');
     packageJson.bin = {
         [context.projectName]: './index.bin.js',
     };
-    fileutils_1.writeJsonFile(`${options.outputPath}/package.json`, packageJson);
+    devkit_1.writeJsonFile(`${options.outputPath}/package.json`, packageJson);
 }
 exports.default = addCliWrapper;
 //# sourceMappingURL=cli.js.map
\ No newline at end of file
diff --git a/node_modules/@simondotm/nx-firebase/src/executors/build/node/package/utils/update-package-json.js b/node_modules/@simondotm/nx-firebase/src/executors/build/node/package/utils/update-package-json.js
index 533b391..32b713a 100644
--- a/node_modules/@simondotm/nx-firebase/src/executors/build/node/package/utils/update-package-json.js
+++ b/node_modules/@simondotm/nx-firebase/src/executors/build/node/package/utils/update-package-json.js
@@ -1,19 +1,19 @@
 "use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
-const fileutils_1 = require("@nrwl/workspace/src/utilities/fileutils");
+const devkit_1 = require("@nrwl/devkit");
 const path_1 = require("path");
 function updatePackageJson(options, context) {
     const mainFile = path_1.basename(options.main).replace(/\.[tj]s$/, '');
     const typingsFile = `${mainFile}.d.ts`;
     const mainJsFile = `${mainFile}.js`;
-    const packageJson = fileutils_1.readJsonFile(path_1.join(context.root, options.packageJson));
+    const packageJson = devkit_1.readJsonFile(path_1.join(context.root, options.packageJson));
     if (!packageJson.main) {
         packageJson.main = `${options.relativeMainFileOutput}${mainJsFile}`;
     }
     if (!packageJson.typings) {
         packageJson.typings = `${options.relativeMainFileOutput}${typingsFile}`;
     }
-    fileutils_1.writeJsonFile(`${options.outputPath}/package.json`, packageJson);
+    devkit_1.writeJsonFile(`${options.outputPath}/package.json`, packageJson);
 }
 exports.default = updatePackageJson;
 //# sourceMappingURL=update-package-json.js.map
\ No newline at end of file
ben-kn-app commented 2 years ago

Thanks for the great patch! Really helped us out!

While using the patch I ran into the problem below

Cannot read property 'type' of undefined
TypeError: Cannot read property 'type' of undefined
    at /node_modules/@simondotm/nx-firebase/src/executors/build/build.js:95:26
    at Array.filter (<anonymous>)
    at /node_modules/@simondotm/nx-firebase/src/executors/build/build.js:94:46
    at Generator.next (<anonymous>)
    at fulfilled (/node_modules/tslib/tslib.js:114:62)

I just dirty fixed it by adding a (dep) check src/executors/build/build.js line 95 return ((dep) && (dep.type === 'lib') && (dep.data.targets['build'] === undefined));

Just leaving it here, in case someone encounters the same problem.

shannonjohnstone commented 2 years ago

Just wanted to ask that since this issue has been open for so long and we have no idea of when it possibly will be fixed, has anyone considered forking this repo implementing that required fixes and publishing a new package rather than all the work arounds?

Honest question as if someone has consider it doing so I was wanting to know what for "cons" have lead to that not happening?

PS. Thanks to everyone in this thread who have contributed to the workarounds 👍

romshiri commented 2 years ago

Did anyone fix it? facing the same issue.

jimlloyd commented 2 years ago

Ideally @simondotm will fix and publish a new release, but I think several of us are unblocked by using the patches provided above. Using patch-package might seem daunting but it is actually quite easy. I'm using nx@13.8.3 with Node 16 and things seem to be working fine.

romshiri commented 2 years ago

@jimlloyd Thanks. Used patch-pacakage and it worked fine. Now I wish I could find a way to make the functions to "hot reload" every time I make a change..

jimlloyd commented 2 years ago

@romshiri Yes that's a problem. It's aggravating because the hot reload does work some (most?) of the time, but I have spent some time debugging a problem longer than necessary because I had not noticed that the hot reload wasn't working. Looking at the project README I see this note:

IMPORTANT: Note that whilst nx serve will be useful when changing existing functions code, due to tsc --watch being enabled, it will NOT correctly detect changes if additional library imports are added to the source code or changes are made to any imported libraries during a watched session. To remedy this, relaunch the emulators by running nx serve again.

Perhaps that spells out exactly when hot reload doesn't work.

romshiri commented 2 years ago

@jimlloyd For me it doesn't work at all. It's a bit a bummer to serve it again on every small change. I'm thinking as a (not so ideal) workaround to create a simple node.js project just for the "rapid development phase" and then copy everything to the function once complete.

jvalentik commented 2 years ago

Hello, I noticed another issue at least on latest NX that copyAssetsFiles is undefined Here is really quick dirt hack added to @ben-kn-app addition to the original patch... @simondotm+nx-firebase+0.3.3.txt

If anyone else stumbles upon this, just rename the file to .patch, drop it to patches folder and apply as per patch-package... Thanks everyone here 👍

dpwanjala commented 1 year ago

Thank you @jvalentik! Faced the copyAssetsFiles is undefined issue while using nx version 14.7.5 and your patch helped fix it.

simondotm commented 1 year ago

Finally released v0.3.4 of the plugin to npm with this fix. Apologies for how long this took to get done.

simondotm commented 1 year ago

Thanks to everyone for their contributions btw. 👍