Skip to content
This repository has been archived by the owner on May 18, 2023. It is now read-only.

Error when deploying with link to local Mira #88

Closed
irelandm opened this issue Sep 7, 2020 · 9 comments
Closed

Error when deploying with link to local Mira #88

irelandm opened this issue Sep 7, 2020 · 9 comments
Assignees
Labels
bug Something isn't working

Comments

@irelandm
Copy link
Collaborator

irelandm commented Sep 7, 2020

Describe the bug
When working on a project with Mira linked locally (folder link or via npm link), the npm run deploy command fails with an error:

Failed: could not deploy the stack Error: unable to determine cloud assembly output directory. Assets must be defined indirectly within a "Stage" or an "App" scope

This makes it hard to work on changes to Mira.

To Reproduce
Steps to reproduce the behavior:

  1. Make a repo from the S3 web hosting sample, or otherwise start a new Mira project
  2. Checkout Mira locally and build it
  3. Change the mira link in your project to point at the local mira
  4. npm install
  5. npm run build
  6. npm run deploy
  7. See error (this stops the deploy)

Expected behavior
The error should not appear and the deploy should succeed

Screenshots
Screenshot 2020-09-07 at 15 33 34

Additional context
May be tied to a particular CDK upgrade. Nigel has reported success with CDK 1.60.0 but I've not yet reproduced that.
Error doesn't occur when pulling mira from npm.

@irelandm irelandm added the bug Something isn't working label Sep 7, 2020
@irelandm
Copy link
Collaborator Author

irelandm commented Sep 8, 2020

Temporary workaround is to use npm pack on mira and then npm install <filename> of the .tgz packed file from your project

@mikegwhit
Copy link
Contributor

Interesting. I'm having difficulty reproducing. Then again, I had trouble utilizing the npx mira because we need to patch a one-line fix for Windows.

While i can continue working to repro this, the first question i may ask is: why are we using npm link?

The error reads as though the Mira code is trying to find a cdk.out folder within the node_modules directory where Mira has been linked to. It's also possible that the setting the --output flag would help. I note we offer the --output-file flag but what's odd is I don't even see this pop up on cdk deploy --help.

Where I also have questions is why npm pack solves this. To me we'd want to figure out where that cdk.out folder is pointing.

@irelandm
Copy link
Collaborator Author

irelandm commented Sep 9, 2020

I had trouble utilizing the npx mira because we need to patch a one-line fix for Windows.

@mikegwhit , I've also seen a bug (presumably the same one) on Windows but thought it was my setup - I had to remove a '.cmd' from one statement, but it seemed to be needed elsewhere. Please go ahead and make an issue ticket for the bug and then fix it.

@irelandm
Copy link
Collaborator Author

irelandm commented Sep 9, 2020

While i can continue working to repro this, the first question i may ask is: why are we using npm link?

In order to make changes to the Mira library while testing that the changes work in a project, so both repos are local to the developer.

Where I also have questions is why npm pack solves this.

npm pack does not solve this, it is a workaround. By packing Mira locally and then pulling the packed file into the local project repo, the Mira code is moved under the project root and avoids the issue of linking to a peer folder/directory (not under the project root)

@irelandm
Copy link
Collaborator Author

irelandm commented Sep 9, 2020

I note we offer the --output-file flag

It's to capture the output of the deployed stack, e.g.
npm run deploy -- --outputs-file ~/mira/upgrade-minutes-test/outputtest

In response to this problem aws/aws-cdk#1773

@mikegwhit
Copy link
Contributor

mikegwhit commented Sep 10, 2020

For the moment, the only discernible fix I can verify is that we copy the local Mira folder into the target application's node_modules. This is not ideal because it requires running some script each time a developer wants to experience new local changes in Mira. Copying is an disk-intensive task.

The reason for this is because (I suspect) you cannot link Mira (or any Node package) and read from the target applications node_modules directory. For most dependencies this is no problem, but with CDK this at least causes TypeScript compilation errors and I suspect is why we're generating the error message above. In addition to using npm link, we also cannot:

  • use ln -s ../mira mira in an application's node_modules folder
  • use npm i ../mira to install a dependency using "mira": "file:../mira"

Other tactics tried were to remove the node_modules folder within the Mira dependency (regardless of method used). Tried to copy peerDependencies in application package.json file. Tried to only symlink the dist folder. These methods all fail because the Node process (when running cli.js to include ../dist/src/cdk/bootstrap.js) will attempt to require based on the resolved location of the symlink, not the location where the symlink exists. This means (I suspect) that symlinked Mira reads @aws-cdk/core in D:/nearform/mira and the --file=someApp.js reads in D:/nearform/mira-application. This means two separate versions (of identical files in different locations) are read into memory, and the references CDK depends on become broken (hence the error).

Aside from copying files (not ideal), there are two strategies:

  • attempt to cajole Node.js to require within the application directory's node_modules folder, not the location of the resolved symlink
  • attempt to diagnose the error message specified above

Due to previously experiencing errors with CDK stemming from this pattern, I'm inclined toward the option of cajoling Node to read from the application's node_modules directory. Ideas include:

  • set the process.cwd within Mira's bootstrapping files
  • configure the Node require object's lookup paths

We can confirm that requiring Mira as a node_module works. It just fails when we require it locally. This pattern of issue was similarly experienced when we implemented Mira previously. We opted for a development strategy that allowed us to:

  • verify the exact build of Mira being used in an application
  • automatically update Mira if there were updates available
  • allow a method for setting the Mira version without fiddling with the package.json file (or lock files)

We also had a development process of developing constructs within the Mira package itself. There wasn't as heavy a need to test outside of the Mira package, or we had some assurance that when we did first test changes that we had a somewhat mirrored environment within the Mira package -- we were assured to some extent that we weren't repeatedly patch-fixing our way to a working build. When we did have to patch-fix, we had a way to get the team up-to-date and assuredly on the same page.

AFAIK, this set of issues is known to the CDK team. I communicated with them back on April on this set of issues. It seemed like the consensus was that CDK was not yet ready to be bundled into an application wrapper (e.g. Mira) and then installed globally. The only reason we have succeeded with Mira as a node_module is because of peer dependencies. As specified, I strongly suspect this issue is related to (but different from) the same set of reasons CDK is temperamental about peer dependencies.

Will try to pursue the above mentioned solutions tomorrow. Otherwise, can pursue a script that hard-copies Mira to a target application on-demand.

@mikegwhit
Copy link
Contributor

mikegwhit commented Sep 11, 2020

I need to boil this down into the barebones of what is needed, but assuming a test.js file with the following contents:
require('mira/dist/src/cdk/app.js');

Then this command works with a symlink'd mira dependency:
node --preserve-symlinks --preserve-symlinks-main node_modules/aws-cdk/bin/cdk deploy --app "node --preserve-symlinks-main --preserve-symlinks --preserve-symlinks-main test.js --file=infra/src/index.js --env=default" --env=default --profile=mira-dev

The secret sauce is --preserve-symlinks which was introduced for the use case of a symlinked package with peerDependencies. Discovered via https://chevtek.io/you-can-finally-npm-link-packages-that-contain-peer-dependencies/

If that command works, then we should be able to modify the bootstrap.ts file to mimic this syntax. For some reason, it seems like even with the --preserve-symlinks-main we have to reference a file outside of a symlinked folder. Here, we can create a temporary file (programmatically), e.g. node_modules/mira-bootstrap.js and this should work.

@mikegwhit
Copy link
Contributor

@mikegwhit
Copy link
Contributor

Today, discovered some aspects of the proposed fix showed as a false positive.

In short:

  • must create a bootstrap file ending in a filename suffix of app.js (see app.js file at bottom)
  • we need to symlink in dist
  • symlink in bin
  • symlink in package.json

Where this is currently going wrong is that Mira still will read from node_modules/mira/node_modules/@aws-cdk/core vs. what's intended of node_modules/@aws-cdk/core. The --preserve-symlinks gets us further, but adding peerDependencies on both application and Mira doesn't seem to cajole the require module in particular versions of Node v14 seemed to work whereas Node v12 did not.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants