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
[aws-lambda-nodejs] Recent changes in the module broke our setup (monorepo, private repos, typescript, lerna) #9032
Comments
What's in the |
So that's our lerna.json:
I have added the lambdas now trying to find a working configuration, it wasn't there before. in package.json the workspaces field is the same now
we have multiple NodejsFunction-Constructs. In lib we have a dozen Stacks and there are mostly directly under lib. So we have infra-cdk/lib/metrics-stack.ts which references a lambda in infra-cdk/lib/lambdas/cloudwatch-ecs-eni-utilization.ts
We need to add the projectRoot, because CodeBuild removes the .git directory |
I have also moved one of the stacks parts in own directory as the file was grown to big: |
And where is this |
it is in packages/aws-utils |
It gets installed as it is typescript and should be compiled, probably I could work around this with references in tsconfig - in fact the infra-cdk itself still has this reference, because in the earlier versions it was its package.json the source of truth for the lambdas |
But Parcel can bundle it even if it's typescript, have you tried that? (remove it from the |
So I have tested this change and it goes through, though it takes longer as earlier, and it changes my yarn.lock and removes much of the packages from it - most notably lerna. So is there something which I could do to stop it from changing the yarn.lock and probably just build faster, but this would be just bonus. |
Having nested packages/workspaces is usually an anti-pattern when working with lerna/yarn workspaces. You should normally have a single |
this was in fact the way it was before I had cdk updated and had the problem with the lambdas not building, will try now to revert some of the changes |
So I have said, that it works but I had another problem, which has conceal the fact that it does not in fact work. So I had codebuild step-function implementation, which used the renderTask and not the new _renderTask interface and that's why after finishing with the lambdas, the problem was that there is no _renderTask method, which could be called.
The files and the directory itself do not exist after the run (and I don't have global aws-cdk installed):
The second thing is that the nodejsfunction build times are now really prohibitive, with a dozen lambdas it needs more than half an hour to complete. |
Looks like somehow something is reinstalling your whole repo at each |
I'm running it locally and yes I have removed all the nodeModules on the last test. |
So now I have tested multiple configurations and settings. The real problem is globally installed/or docker installed parcel-2 and yarn workspaces/lerna. One comment I have seen in the parcel issues has given me a clue for a workaround parcel-bundler/parcel#1588 (comment). Adding parcel and @babel/core to the root-package devDependencies has solved the problem with the continuous overwriting of yarn.lock and node_modules, and has allowed me to rund cdk diff to an end (I'm still to test if all works ok after deployment, but it looks ok). This workaround seems really strange and more of a hack, then a permanent solution. The speed was somewhat improved with the change, but it's still prohibitive slow - almost a minute pro lambda. The last problem with the private package in the nodeModule will not work with the current implementation, as it runs out of the yarn workspaces/lerna and does not know anything about the private package. It would have been useful at least for me, because sometimes I need to look at the lambda's code in the aws console, and a bundled Lambda is really big, but if a solution could not be implemented, probably a line in the documentation explaining the problem with the private packages would be fine. So I'm not sure how to proceed with this bug, for me it is partially an upstream bug, although there was a functionality in previous versions, which does not work anymore with the current version and I don't see the update as a step forward as besides this bug, the speed of the new parcel bundling is painfully slow. |
So I have now run test against the parcel1 implementation (I have used 5491922 commit) and here are the times from both runs:
The first one is with parcel1 and the second is with parcel2. The times with parcel2 are worse but not that much as I expected. Probably the problem with the times is in Docker and some sort of caching. |
So I now have the time for parcel1 without Docker (I have used 5b34ccb commit)
So I don't know - probably in this case, I'll just package this, probably move to parcel2 and call it a day. |
@gergan I made some tests and I can confirm that there are issues in some monorepo setups (overwriting of
This is the correct temporary workaround, though adding |
When Parcel cannot find `@babel/core` in the search paths for node modules it tries to install it. This overwrites the lock file and `node_modules`. Add the `--no-autoinstall` flag to prevent Parcel from installing any missing dependency and switch to a local install in the Docker image at the root (`/`) of the filesystem. This way all dependencies in `/node_modules` will be in the search paths of `/asset-input`. This was not detected in this repo or in the init template because `@babel/core` is a dependency of `jest`. Closes aws#9032
@gergan can you give it a try with this branch https://github.com/jogold/aws-cdk/tree/parcel-no-auto-install / commit jogold@fcd9979? It should work without any workaround in your root package.json (the first run could take some time because the Docker image must be rebuilt). Also this branch includes #9000 so bundling should be a little faster. |
So I've tested it and unfortunately it didn't help. I think I've tested the no install option to no avail yesterday with a global parcel instance. |
What did you test exactly? Any error messages? |
Sorry, for all the tests I have used an in-tree package, where I just copied the files. Which I did also for your change, but I now see, that I forgot to exchange the imports - as I'm changing between different branches the whole day ... I'll retest ASAP and report here. |
When Parcel cannot find `@babel/core` in the search paths for node modules it tries to install it. This overwrites the lock file and `node_modules`. Add the `--no-autoinstall` flag to prevent Parcel from installing any missing dependency and switch to a local install in the Docker image at the root (`/`) of the filesystem. This way all dependencies in `/node_modules` will be in the search paths of `/asset-input`. This was not detected in this repo or in the init template because `@babel/core` is a dependency of `jest`. Closes aws#9032
No problem, this should be the fix master...jogold:parcel-no-auto-install |
Ok, so it worked and it is a little bit faster on first run
the second run is around 5:30 minutes |
When Parcel cannot find `@babel/core` in the search paths for node modules it tries to install it. This overwrites the lock file and `node_modules`. Add the `--no-autoinstall` flag to prevent Parcel from installing any missing dependency and switch to a local install in the Docker image at the root (`/`) of the filesystem. This way all dependencies in `/node_modules` will be in the search paths of `/asset-input`. This was not detected in this repo or in the init template because `@babel/core` is a dependency of `jest`. Closes #9032 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
When Parcel cannot find `@babel/core` in the search paths for node modules it tries to install it. This overwrites the lock file and `node_modules`. Add the `--no-autoinstall` flag to prevent Parcel from installing any missing dependency and switch to a local install in the Docker image at the root (`/`) of the filesystem. This way all dependencies in `/node_modules` will be in the search paths of `/asset-input`. This was not detected in this repo or in the init template because `@babel/core` is a dependency of `jest`. Closes aws#9032 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Reproduction Steps
So it could be misconfiguration on our end, but recent changes most probably the ones in #8837 broke our setup and I could not find a running configuration for now.
so we have a monorepo with lerna, yarn workspaces, only typescript and the following structure
So we have only one node_modules and yarn.lock and generally the code before the changes used to find the node_modules which was there in the root of the project, our code has always set the projectRoot to the root of the project and in this way all has worked.
We didn't have package.json in the lambdas directory, which somehow does not work now - so I've created one.
Now I have a code state with changes which partially work if I
This results in some of the lambdas have been built and probably the second one (using our private package) is broken with the message, which is in Error Log Section. Parcel tries to find our package, but It is not there. I should mention, that there are some other problems, which correlate with this issue - after this error the node_modules is missing some modules as for example lerna and I need to make yarn install again in order to work with repo. So my assumption is that the code uses now erroneously the lambda package.json but installs in the root directory, which leads to this problem.
Error Log
Environment
Other
If this is simply a configuration issue I'll be glad to test some ideas or advices on how to handle this, or if you need more information please tell me.
This is 🐛 Bug Report
The text was updated successfully, but these errors were encountered: