fix: error if root typing path is invalid #557
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #557 +/- ##
===========================================
- Coverage 93.99% 93.80% -0.20%
===========================================
Files 42 42
Lines 2414 2420 +6
Branches 500 501 +1
===========================================
+ Hits 2269 2270 +1
- Misses 145 150 +5 |
| if (typeof rootType !== 'string') { | ||
| const rootTypePath = rootType.path | ||
|
|
||
| if (typeof rootTypePath !== 'string' || !path.isAbsolute(rootTypePath)) { | ||
| throw new Error( | ||
| `Expected an absolute path for the root typing path of the type ${typeName}, saw ${rootTypePath}` | ||
| ) | ||
| } | ||
|
|
||
| if (!fs.existsSync(rootTypePath)) { | ||
| throw new Error(`Root typing path ${rootTypePath} of the type ${typeName} does not exist`) | ||
| } | ||
|
|
||
| const importPath = relativePathTo(rootType.path, outputPath) | ||
| .replace(/(\.d)?\.ts/, '') | ||
| .replace(/\\+/g, '/') | ||
|
|
||
| importMap[importPath] = importMap[importPath] || new Set() | ||
| importMap[importPath].add(val.alias ? `${val.name} as ${val.alias}` : val.name) | ||
| importMap[importPath].add(rootType.alias ? `${rootType.name} as ${rootType.alias}` : rootType.name) |
There was a problem hiding this comment.
This is technically a breaking change as I'm now only accepting absolute file paths, to ensure the ability to check whether the file exists.
@tgriesser I'm not sure what was the use-case for enabling any kind of path. Path aliases maybe? Do you feel strongly about keeping any string allowed? In that case, I'll make the checks only in case the path is absolute.
The reason I made it this way is that we're already that strict when configuring a typegenAutoConfig source.
There was a problem hiding this comment.
LGTM, let's wait a bit to see if @tgriesser has thoughts too.
|
@Weakky since there were no tests docs or comments to indicate otherwise I think we can proceed with this PR. |
|
@jasonkuhrt I was planning to mention the breaking change in the squashed commit |
|
@Weakky I don't think we have to consider them breaking because it was never documented or tested for that use-case. But you're right that it would be nice to mention. Let's add some detail in the release notes when we cut stable? |
Closes #396
We unfortunately cannot add a runtime error if the type that's imported from the path does not exist without substantial work and/or performance overhead.
If we use the current regex approach, we might end up throwing in cases where the type is properly exported but the regex fails to see that.
The only approach would be to parse the actual file with the type-checker, which is way out of scope for this PR.