simondotm / nx-firebase

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

Build command is broken with @nrwl/workspace >= 13.10.0 #48

Closed hendrickson-tyler closed 1 year ago

hendrickson-tyler commented 2 years ago

When running the build command through NX when @nrwl/workspace is installed with a version of 13.10.0 or greater, the following error occurs:

assets_1.copyAssetFiles is not a function

It seems as though the copyAssetFiles function was removed from @nrwl/workspace/src/utilities/assets in 13.10.0, looking at this commit. I'm not sure if this is an intentional change on their part; I can't seem to find any notes on it.

Tested with both 13.10.0 and 13.10.1 (the latest at time of writing). This doesn't appear to be addressed as part of #45, so I figured I would open a separate issue for this.

pascalbe-humanize commented 2 years ago

I can reproduce this error.

anandsathe67 commented 2 years ago

Changing line 74 of src/executors/build/build.js to yield Promise.all(normalizedOptions.files.map((file) => copy(file.input, file.output))); instead of assets_1.copyAssetFiles .... and adding an import const { copy } = require("fs-extra/lib/copy");

fixes this issue

anandsathe67 commented 2 years ago

@simondotm+nx-firebase+0.3.3.patch.txt

This is a cumulative patch for this issue and #44

anandsathe67 commented 2 years ago

@simondotm @BraunreutherA please review

drmikecrowe commented 2 years ago

So, does https://github.com/nrwl/nx/pull/9086 break a bunch of stuff here? I was trying to run on 13.10, but I'm thinking there are some major changes needed?

What versions of nx can we lock down to still use this plugin?

anandsathe67 commented 2 years ago

@drmikecrowe Am using 13.10.2 - other than a patch for this plugin and switching over to using the js:tsc executor instead of the package executor, things work ok

drmikecrowe commented 2 years ago

@anandsathe67 -- see https://gist.github.com/drmikecrowe/8bcfc88a0ee29d6c3535e6460c014144 -- with the recent https://github.com/simondotm/nx-firebase/pull/45, I don't think the patch is needed now. However, I'm still getting problems (yeah, this should have gone on #44, sorry).

Regarding your executor change, do you mean:

"executor": "@simondotm/nx-firebase:build",

to

"executor": "@nrwl/js:tsc",

?

anandsathe67 commented 2 years ago

@anandsathe67 -- see https://gist.github.com/drmikecrowe/8bcfc88a0ee29d6c3535e6460c014144 -- with the recent #45, I don't think the patch is needed now. However, I'm still getting problems (yeah, this should have gone on #44, sorry).

Regarding your executor change, do you mean:

"executor": "@simondotm/nx-firebase:build",

to

"executor": "@nrwl/js:tsc",

?

@nrwl/node:package -> @nrwl/js:tsc.

I was using this executor in my workspace json and hence had to make the change when I migrated to nx 13.x Not sure why patch-package is telling you the patch isnt required. The issues you are seeing in your gist are the ones which the patch (which I had earlier uploaded to #44) includes. I did not see this when i applied it to #45 - the patch did get applied

simondotm commented 2 years ago

I've been reviewing the latest nx code for 13.10.x this afternoon and it seems there's been a fair few changes since I last released this plugin against nx 12.x

Most significantly they've merged the node:package functionality into the js:tsc package so I'm going to need to take a good look over that to bring the plugin code upto spec with the latest nx code.

The patches and comments above have been really useful, thanks everyone.

That said, I dont (yet) see how I can use them in the plugin source code - it really feels like I need to update the plugin to use the latest nx plugin apis, so thats what'll I'm doing atm.

hendrickson-tyler commented 2 years ago

@simondotm+nx-firebase+0.3.3.patch.txt

This is a cumulative patch for this issue and #44

Thanks @anandsathe67, the patch works beautifully! Looking forward to an official fix, thanks to everyone contributing to this.

adamstret commented 1 year ago

@anandsathe67 can you please share with us how to apply the patch? Do I just drop that txt file somewhere in the node_modules in the project? Or do I have to run it somehow? Thanks.

ALSO, PLEASE SHARE: petition for an official Firebase/Nx plugin

anandsathe67 commented 1 year ago

@anandsathe67 can you please share with us how to apply the patch? Do I just drop that txt file somewhere in the node_modules in the project? Or do I have to run it somehow? Thanks.

ALSO, PLEASE SHARE: petition for an official Firebase/Nx plugin

Hi - please see https://www.npmjs.com/package/patch-package - since I have already generated the package you could download it - rename it appropriately and use it. You will have to make the modifications to package.json and install patch_package though as mentioned in the link. Hope that helps

adamstret commented 1 year ago

@anandsathe67 way too complicated for me :( guess I will have to wait until this plugin gets fixed, or until some good soul forks & fixes it... :(

snorreks commented 1 year ago

Just created a package to help with deploying cloud functions for firebase https://www.npmjs.com/package/nx-cloud-functions-deployer. It is heavy opinionated with how you structure the project, but with this approach each function will have separate index file that will reduce size (by using esbuild). Feedback and help would be appreciated 👍

dkhunt27 commented 1 year ago

what I did to get anandsathe67 patch-package working:

now whenever you run yarn install, you should see in the output something from patch-package like so

yarn install v1.22.15 [1/4] 🔍 Resolving packages... success Already up-to-date. $ patch-package patch-package 6.4.7 Applying patches... @simondotm/nx-firebase@0.3.3 ✔ ✨ Done in 1.40s.

simondotm commented 1 year ago

This appears to be fixed in v0.3.4, tested running in Nx 13.10.6

hendrickson-tyler commented 1 year ago

@simondotm Can confirm, the build command is working for me too with Nx 15.2.3. Thanks for the fix!