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

Custom resource build should use the build npm script specified in its package.json #11889

Open
2 tasks
josefaidt opened this issue Feb 2, 2023 · 2 comments · May be fixed by #11854
Open
2 tasks

Custom resource build should use the build npm script specified in its package.json #11889

josefaidt opened this issue Feb 2, 2023 · 2 comments · May be fixed by #11854
Labels
extensibility Issues related to expand or customize current configuration feature-request Request a new feature p3 platform-build Issues related to building resources with `amplify build`

Comments

@josefaidt
Copy link
Contributor

josefaidt commented Feb 2, 2023

Is this feature request related to a new or existing Amplify category?

No response

Is this related to another service?

No response

Describe the feature you'd like to request

instead of invoking node_modules/.bin/tsc directly to build custom resources, we should instead use the defined build script in the resource's package.json. This script is automatically added when we add a custom resource with amplify add custom

ref #11851

Describe the solution you'd like

see above

Describe alternatives you've considered

n/a

Additional context

No response

Is this something that you'd be interested in working on?

  • 👋 I may be able to implement this feature request

Would this feature include a breaking change?

  • ⚠️ This feature might incur a breaking change
@josefaidt josefaidt added pending-triage Issue is pending triage feature-request Request a new feature extensibility Issues related to expand or customize current configuration platform-build Issues related to building resources with `amplify build` and removed pending-triage Issue is pending triage labels Feb 2, 2023
acusti added a commit to acusti/amplify-cli that referenced this issue Feb 7, 2023
…rather than invoking tsc directly from the node_modules/.bin directory
@josefaidt josefaidt added the p3 label Feb 7, 2023
@josefaidt
Copy link
Contributor Author

Note for fix: ensure all other relative paths looking for tsc are also updated (in the case of overrides), or ensure the requested change will support overrides functionality

acusti added a commit to acusti/amplify-cli that referenced this issue Mar 24, 2023
…rather than invoking tsc directly from the node_modules/.bin directory
brianlenz added a commit to brianlenz/amplify-cli that referenced this issue Jul 18, 2024
Updated the TypeScript compilation of overrides so that it doesn't require `node_modules/.bin/tsc`.
Instead, it simply relies on the `build` script to execute `tsc`.
This is more flexible and can support alternative setups w/ hoisting (e.g. via Yarn workspaces).

This is an override corollary fix to aws-amplify#11854, which is for custom resources.

aws-amplify#11889
@brianlenz
Copy link

@josefaidt we are running into the same issue with api overrides (which has code with the identical root issue as here). We use Yarn (4.x) workspaces with hoisting. The Amplify-managed functions in amplify/backend/function/xxx/src are Yarn workspaces. We just added the api override, which has introduced this issue. When we attempt amplify build --debug, we get:

$ amplify build --debug
Overrides functionality is not implemented for this category
🛑 InvalidOverrideError: Packaging overrides failed.
    at /snapshot/amplify-cli/build/node_modules/@aws-amplify/amplify-category-api/lib/index.js:309:23
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async transformCategoryStack (/snapshot/amplify-cli/build/node_modules/@aws-amplify/amplify-category-api/lib/index.js:308:29)
    at async transformResourceWithOverrides (/snapshot/amplify-cli/build/node_modules/@aws-amplify/amplify-provider-awscloudformation/lib/override-manager/transform-resource.js:66:9)
    at async Object.run (/snapshot/amplify-cli/build/node_modules/@aws-amplify/cli-internal/lib/commands/build.js:27:13)
    at async Object.executeAmplifyCommand (/snapshot/amplify-cli/build/node_modules/@aws-amplify/cli-internal/lib/index.js:194:9)
    at async executePluginModuleCommand (/snapshot/amplify-cli/build/node_modules/@aws-amplify/cli-internal/lib/execution-manager.js:139:5)
    at async executeCommand (/snapshot/amplify-cli/build/node_modules/@aws-amplify/cli-internal/lib/execution-manager.js:37:9)
    at async Object.run (/snapshot/amplify-cli/build/node_modules/@aws-amplify/cli-internal/lib/index.js:121:5)
🛑 There was an error building the resource

The issue is the same "TypeScript executable not found." error (though that message isn't properly being exposed in the CLI output). The only workaround I've found to getting this working is to disable hoisting in amplify/backend/package.json:

"installConfig": {
  "hoistingLimits": "workspaces"
}

We are doing the exact same thing as @acusti (#11851 (comment)): Use hoisting to prevent node_modules from being deployed to our functions + using ESBuild via the amplify:functioname Yarn build script. We've taken it a step further and added detection of node_modules in the function directories, in which case it will error out because we never want to package the Amplify functions with node_modules to avoid bloat.

It seems that the design of overrides is less-than-desirable in that the package.json and tsconfig.json live in amplify/backend dir, which is a parent of /amplify/backend/function. This makes it a bit more complex to use Yarn workspaces & hoisting because the function workspaces will "inherit" the config from the package.json in amplify/backend, which isn't desirable. In this case, when we disable hoisting for amplify/backend (config shown above), that configuration cascades down to all of the functions and results in node_modules being included for each, which we need to avoid.

The root cause of the problem is identical to what is being fixed in #11854. The Amplify CLI should not directly rely on node_modules/.bin/tsc, but should instead abstract the location of the tsc executable to the build script.

I have created #13858 to fix the issue in the exact same way. Would love to see these PRs merged & released so we don't have to use custom amplify-dev builds!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extensibility Issues related to expand or customize current configuration feature-request Request a new feature p3 platform-build Issues related to building resources with `amplify build`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants