-
-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Build command is broken with @nrwl/workspace >= 13.10.0 #48
Comments
I can reproduce this error. |
Changing line 74 of src/executors/build/build.js to fixes this issue |
@simondotm+nx-firebase+0.3.3.patch.txt This is a cumulative patch for this issue and #44 |
@simondotm @BraunreutherA please review |
So, does nrwl/nx#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? |
@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 |
@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:
to
? |
@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 |
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. |
Thanks @anandsathe67, the patch works beautifully! Looking forward to an official fix, thanks to everyone contributing to this. |
@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: |
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 |
@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... :( |
Just created a package to help with deploying cloud functions for firebase https://www.npmjs.com/package/nx-cloud-functions-deployer. |
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 |
This appears to be fixed in v0.3.4, tested running in Nx 13.10.6 |
@simondotm Can confirm, the build command is working for me too with Nx 15.2.3. Thanks for the fix! |
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:
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.
The text was updated successfully, but these errors were encountered: