aws-amplify / aws-amplify/amplify-cli

Custom transformers: improve documentation, allow file:// to work with relative paths

Open
#10,899 0 comments 2 reactions 0 assignees View on GitHub
feature-request p4 platform platform-plugin
Dominant language
TypeScript
Stars
2.9k
Forks
825
Avg merge
11d 23h
Merged PRs (30d)
2

Description

### 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

Hello Amplify team!

My org uses a couple of custom transformers, and I'm tweaking them for the first time in a while. I've noticed a deficiency in your documentation located here: https://docs.amplify.aws/cli/plugins/authoring/#adding-custom-graphql-transformers-to-the-project

The documentation says:

> A transformer can be registered by adding a file URI to the JavaScript file that implements the transformer or by specifying the npm package name. The transformer modules will be dynamically imported during the transform process.

The deficiency is that it fails to note that the process which loads up the transformer modules will try to import packages that are not file-URI format using normal node resolution before it tries to download them as global packages from NPM.

I have found the code where this used to happen in an old PR: https://github.com/aws-amplify/amplify-cli/pull/9252/files#diff-f19fff3a749d53b626fe4cba50325e9538cb833636acc889419f8445bf93df1a -- the code there looks like this:

```typescript
const customTransformers = (Array.isArray(customTransformerList) ? customTransformerList : [])
.map(transformer => {
const fileUrlMatch = /^file:\/\/(.*)\s*$/m.exec(transformer);
const modulePath = fileUrlMatch ? fileUrlMatch[1] : transformer;
if (!modulePath) {
throw new Error(`Invalid value specified for transformer: '${transformer}'`);
}
// The loading of transformer can happen multiple ways in the following order:
// - modulePath is an absolute path to an NPM package
// - modulePath is a package name, then it will be loaded from the project's root's node_modules with createRequireFromPath.
// - modulePath is a name of a globally installed package
let importedModule;
const tempModulePath = modulePath.toString();
try {
if (path.isAbsolute(tempModulePath)) {
// Load it by absolute path
importedModule = require(modulePath);
} else {
const projectRootPath = context.amplify.pathManager.searchProjectRootPath();
const projectNodeModules = path.join(projectRootPath, 'node_modules');
try {
importedModule = importFrom(projectNodeModules, modulePath);
} catch (_) {
// Intentionally left blank to try global
}
// Try global package install
if (!importedModule) {
importedModule = importGlobal(modulePath);
}
}
// At this point we've to have an imported module, otherwise module loader, threw an error.
return importedModule;
} catch (error) {
context.print.error(`Unable to import custom transformer module(${modulePath}).`);
context.print.error(`You may fix this error by editing transformers at ${path.join(resourceDir, TRANSFORM_CONFIG_FILE_NAME)}`);
throw error;
}
})
```

However, that file doesn't even exist anymore. Looking at [the history for the containing directory](https://github.com/aws-amplify/amplify-cli/commits/dev/packages/amplify-provider-awscloudformation/src), it seems like it was moved out into the API category, but then maybe moved out again, possibly extracted into its own package? I lost the thread at that point. If you could point me to the new location, and better yet, add a link to that source code into the documentation I linked above, I would appreciate it.

However, based on my own experimentation, it seems like wherever this code now lives, it must not have changed much if at all. It does the following:

1. Strips off the `file://` prefix if present
2. Uses `path.isAbsolute` to determine whether the remaining string (which may never have had a `file://` prefix) is an absolute path on the system, and `require`s it if so
3. Otherwise, tries to `require` it from within `/node_modules` <-- this is the part that is undocumented
4. If that doesn't work, uses the `importGlobal` function, which I haven't looked into, but based on the documentation and some other experiments I did, I think it checks for a global npm package already installed with the name, and if it doesn't find one, it tries to download one from the registry

### Describe the solution you'd like

First of all, whatever ends up happening, it would be very helpful if the documentation accurately reflected it, and ideally had a link to the source code - anyone writing a custom transformer would likely appreciate the ability to really understand how it is going to be loaded by going straight to the source.

Second, I think the resolution algorithm should actually be changed a bit. It would be better if it worked more like the following:

Constraints:
1. If the entry starts with `file://` it is always treated as a local filepath, and if one cannot be resolved, it should fail rather than attempting to import from any local `node_modules` or download anything from a registry.
2. Ideally, one should be able to use relative paths with the file URI option. Absolute paths are pretty much useless on my team and I suspect many others, because no other dev on my team is going to have a `/Users/dan/development/` directory, and the CI pipeline certainly won't.

Implementation pseudocode:
```typescript
const fileUrlMatch = /^file:\/\/(.*)\s*$/m.exec(transformer);
if (fileUrlMatch) {
let fileSystemPath = fileUrlMatch[1];
if (!path.isAbsolute(fileSystemPath)) {
const projectRootPath = context.amplify.pathManager.searchProjectRootPath();
fileSystemPath = path.join(projectRootPath, fileSystemPath)
}
importedModule = require(fileSystemPath);
} else {
// behaves much as before, looking in the project's node_modules then moving on to npm global packages and finally checking the registry and downloading and caching the package if needed
}
```

### Describe alternatives you've considered

This is relevant for us because our packages are part of our private source code, not available on NPM, and we want to keep them in a monorepo (rather than something like an NPM private package, which would create a host of problems around managing access tokens and things, especially painful in CI).

What we actually do is make use of the undocumented behavior of looking in the project's root `node_modules` by using `yarn` workspaces, which create a symlink in the project's root `node_modules` to the actual definition of the package which lives in a `packages/` directory.

I think it would be better for the project if we could put `file://packages/custom-transformer-name` in the `transform.conf.json` file though, rather than needing to rely on `yarn` workspace behavior.

### Additional context

I've selected "This feature might incur a breaking change" because I'm asking for a fail-fast approach where `file://`-prefixed entries will no longer work at all if the filepath doesn't exist on the local filesystem, rather than the current behavior of treating `file://some-package-name` and `some-package-name` exactly the same. It's possible some project is relying on that odd behavior.

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

- [X] 👋 I may be able to implement this feature request

### Would this feature include a breaking change?

- [X] ⚠️ This feature might incur a breaking change

Contributor guide

Open the contributing guide

Research direction

Start with the custom transformer documentation at docs.amplify.aws/cli/plugins/authoring/#adding-custom-graphql-transformers-to-the-project and trace the loader from the old amplify-cli PR #9252, since the issue says the referenced file moved. Review transform.conf.json handling and confirm the current resolution behavior. Done means the documentation links to the relevant source and accurately describes package and file:// resolution, including relative paths and failure behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
cli, documentation
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.