feat(core): throw graceful error in runNxMigration() if no collection #10655
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
trakhimenok
changed the title
There was a problem hiding this comment.
Hey @trakhimenok! Some better messaging in this case would definitely make sense, although it is likely an issue caused by the migration's author.
We recently introduced plugin lint checks that should reduce the likelihood of this occurring, but better messaging still makes sense.
I think try ... catch may be the wrong control flow for this though, do you care to take a look at my suggestion?
| try { | ||
| const c = g[name]; | ||
| implRelativePath = c.implementation || c.factory; | ||
| } catch (e) { | ||
| throw new Error( | ||
| `Can not get relative path for migration's implementation as collection not found by name="${name}" for package="${packageName}", collectionPath="${collectionPath}"` | ||
| ); | ||
| } |
There was a problem hiding this comment.
Rather than a try catch here, we could do something like this:
| try { | |
| const c = g[name]; | |
| implRelativePath = c.implementation || c.factory; | |
| } catch (e) { | |
| throw new Error( | |
| `Can not get relative path for migration's implementation as collection not found by name="${name}" for package="${packageName}", collectionPath="${collectionPath}"` | |
| ); | |
| } | |
| const c = g[name]; | |
| implRelativePath = g[name]?.implementation || g[name]?.factory | |
| if (!implRelativePath) { | |
| throw new Error( | |
| `Unable to determine relative path for migration implementation: { "name": "${name}", "package": "${packageName}", "collectionPath": "${collectionPath}"` | |
| ); | |
| } |
There was a problem hiding this comment.
@AgentEnder addressed
| const implRelativePath = g[name]?.implementation || g[name]?.factory; | ||
| if (!implRelativePath) { | ||
| throw new Error( | ||
| `Can not get relative path for migration's implementation as collection not found by name="${name}" for package="${packageName}", collectionPath="${collectionPath}"` |
There was a problem hiding this comment.
This messaging is still not quite accurate. The collection was found, but the implementation was not. I'd recommend something like this:
| `Can not get relative path for migration's implementation as collection not found by name="${name}" for package="${packageName}", collectionPath="${collectionPath}"` | |
| `Unable to determine implementation path for ${collectionPath}:${name}` |
The collection:generator pattern matches expectations from nx g ..., so I think its more readable.
There was a problem hiding this comment.
Have a look at new message? I've added also indication if generators or schematics was used.
And better error message
as requested in PR review
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
This will give better context to user when getting an error described in "current behaviour".
Current Behavior
For some unclear reason getting an error when running
pnpm exec nx migrate --run-migrationsExpected Behavior
Expected to get context on where is the problem so might be able to fix.
Related Issue(s)
Fixes #