fix: remove vm2 #12989
fix: remove vm2 #12989
Conversation
| formatter.list(['No override File Found', `To override ${this.resourceName} run amplify override auth`]); | ||
| return ''; | ||
| }); | ||
|
|
||
| const sandboxNode = new vm.NodeVM({ | ||
| console: 'inherit', | ||
| timeout: 5000, | ||
| sandbox: {}, | ||
| require: { | ||
| context: 'sandbox', | ||
| builtin: ['path'], | ||
| external: true, | ||
| }, | ||
| }); | ||
| const overrideJSFilePath = path.join(overrideDir, 'build', 'override.js'); | ||
| const projectInfo = getProjectInfo(); | ||
| try { | ||
| await sandboxNode | ||
| .run(overrideCode, path.join(overrideDir, 'build', 'override.js')) | ||
| .override(this._authTemplateObj as AmplifyAuthCognitoStack & AmplifyStackTemplate, projectInfo); | ||
| const overrideImport = await import(overrideJSFilePath); | ||
| if (overrideImport && overrideImport?.override && typeof overrideImport?.override === 'function') { | ||
| overrideImport.override(this._authTemplateObj, projectInfo); | ||
| } |
There was a problem hiding this comment.
now that all these overrides look very similar consider extracting common code somewhere here https://github.com/aws-amplify/amplify-cli/tree/dev/packages/amplify-cli-core/src/overrides-manager . e.g. create override-runner.ts and extract code in such a way that what remains here is await runOverride(overrideDir, this._authTemplateObj).
Project info can be looked up in core I think, if not add one more parameter.
yarn.lock
Outdated
| @@ -29815,7 +29812,7 @@ node-pty@beta: | |||
| languageName: node | |||
| linkType: hard | |||
|
|
|||
| "vm2@npm:^3.9.19, vm2@npm:^3.9.3, vm2@npm:^3.9.8": | |||
| "vm2@npm:^3.9.3, vm2@npm:^3.9.8": | |||
There was a problem hiding this comment.
I suggest we get this into dev first and merge to this PR #12986 .
So that we can remove vm2 entries entirely.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| export const runOverride = (overrideDir: string, templateObject: $TSAny, projectInfo: $TSAny): void => { | ||
| const overrideJSFilePath = path.join(overrideDir, 'build', 'override.js'); | ||
| delete require.cache[require.resolve(overrideJSFilePath)]; | ||
| const overrideImport = require(overrideJSFilePath); | ||
| if (overrideImport && overrideImport?.override && typeof overrideImport?.override === 'function') { | ||
| overrideImport.override(templateObject, projectInfo); | ||
| } | ||
| }; |
There was a problem hiding this comment.
In some places we had.
await sandboxNode
.run(overrideCode, overrideJSFilePath)
.override(this._resourceTemplateObj as AmplifyDDBResourceTemplate, projectInfo);
we should make this function async and await overrideImport.override(templateObject, projectInfo);.
This works even if overrideImport.override is sync and returns void . The way compiler handles awaits is that it inspects return object coming from function at runtime.
| @@ -30715,7 +30712,7 @@ node-pty@beta: | |||
| languageName: node | |||
| linkType: hard | |||
|
|
|||
| "vm2@npm:^3.9.19, vm2@npm:^3.9.3": | |||
| "vm2@npm:^3.9.3": | |||
There was a problem hiding this comment.
I was expecting this to be gone.
Could you yarn why vm2 --recursive to see where this is still coming from ?
There was a problem hiding this comment.
Looks like.
$ yarn why vm2
└─ degenerator@npm:3.0.1
└─ vm2@npm:3.9.19 (via npm:^3.9.3)
$ yarn why degenerator
└─ pac-resolver@npm:5.0.0
└─ degenerator@npm:3.0.1 (via npm:^3.0.1)
$ yarn why pac-resolver
└─ pac-proxy-agent@npm:5.0.0
└─ pac-resolver@npm:5.0.0 (via npm:^5.0.0)
$ yarn why pac-proxy-agent
└─ proxy-agent@npm:5.0.0
└─ pac-proxy-agent@npm:5.0.0 (via npm:^5.0.0)
$ yarn why proxy-agent
├─ @aws-amplify/amplify-category-notifications@workspace:packages/amplify-category-notifications
│ └─ proxy-agent@npm:5.0.0 (via npm:^5.0.0)
│
├─ @aws-amplify/amplify-cli-core@workspace:packages/amplify-cli-core
│ └─ proxy-agent@npm:5.0.0 (via npm:^5.0.0)
│
└─ @aws-amplify/amplify-provider-awscloudformation@workspace:packages/amplify-provider-awscloudformation
└─ proxy-agent@npm:5.0.0 (via npm:^5.0.0)
There was a problem hiding this comment.
So we should seek proxy-agent update. But that should happen in separate PR.
There was a problem hiding this comment.
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## dev #12989 +/- ##
===========================================
+ Coverage 0.00% 48.39% +48.39%
===========================================
Files 1296 841 -455
Lines 149743 38029 -111714
Branches 1296 7781 +6485
===========================================
+ Hits 0 18406 +18406
+ Misses 148447 18031 -130416
- Partials 1296 1592 +296
... and 1234 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
awsluja
added
the
do-not-merge
awsluja
removed
the
do-not-merge
Description of changes
Remove dependency on vm2. Remaining references should be removed with API changes
Issue #, if available
Description of how you validated changes
Checklist
yarn testpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.