Skip to content
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] when interrupting cdk commands (synth), package.json is left "dirty" #9130

Closed
vvo opened this issue Jul 17, 2020 · 3 comments · Fixed by #11289
Closed
Assignees
Labels
@aws-cdk/aws-lambda-nodejs bug This issue is a bug. effort/small Small work item – less than a day of effort in-progress This issue is being actively worked on. p2

Comments

@vvo
Copy link
Contributor

vvo commented Jul 17, 2020

Hi there, when using aws-lambda-nodejs, one of the step of the program is to add some configuration for parcel inside the user package.json:

  "cdk-lambda": "/asset-output/index.js",
  "targets": {
    "cdk-lambda": {
      "context": "node",
      "includeNodeModules": {
        "aws-sdk": false
      },
      "sourceMap": false,
      "minify": false,
      "engines": {
        "node": ">= 12"
      }
    }
  }

I can see that when running cdk synth, my package.json gets updated and then the configuration is removed.

The issue is that if you interrupt cdk synth, for example if it is slow with CTRL+C in a terminal, and it has not yet finished what it needs to do, then your package.json is left dirty with all the parcel configuration still present in it.

I first thought it was some configuration that was needed to be here every time, and then understood it's only temporary configuration.

Reproduction Steps

  • use aws-lambda-nodejs
  • launch cdk synth
  • stop it in the middle with ctrl + c

Error Log

no errors

Environment

  • CLI Version : 1.5.1
  • Framework Version: 1.5.1
  • Node.js Version: 12.18.2
  • OS : 10.15.5
  • Language (Version): JavaScript

Other

n/a


This is 🐛 Bug Report

@vvo vvo added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 17, 2020
@eladb
Copy link
Contributor

eladb commented Jul 20, 2020

Copy @jogold - any way we can avoid writing package.json and perhaps reference a copy instead? It it possible to tell parcel to look for package.json elsewhere?

@eladb eladb added the p2 label Jul 20, 2020
@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Jul 20, 2020
@eladb eladb added the effort/small Small work item – less than a day of effort label Aug 4, 2020
@jogold
Copy link
Contributor

jogold commented Aug 17, 2020

@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Nov 5, 2020
eladb pushed a commit that referenced this issue Nov 18, 2020
Replace Parcel with esbuild for bundling.

esbuild offers [impressive performances](https://esbuild.github.io/) compared to Parcel.
Moreover everything can be configured via the CLI. This means that
we don't  need to play with the user `package.json` file anymore.

Add full Windows support for local bundling.

Refactor and clean-up.

Closes #10286
Closes #9130
Closes #9312
Resolves #11222

BREAKING CHANGE: local bundling now requires `esbuild` to be installed.
* **lambda-nodejs**: `projectRoot` has been replaced by `depsLockFilePath`. It should point to your dependency lock file (`package-lock.json` or `yarn.lock`)
* **lambda-nodejs**: `parcelEnvironment` has been renamed to `bundlingEnvironment`
* **lambda-nodejs**: `sourceMaps` has been renamed to `sourceMap`
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda-nodejs bug This issue is a bug. effort/small Small work item – less than a day of effort in-progress This issue is being actively worked on. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants