-
Notifications
You must be signed in to change notification settings - Fork 104
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
fix: tolerate an undefined baseUrl
compiler option
#208
Conversation
38e04a8
to
6a88d79
Compare
@@ -22,7 +22,7 @@ export interface ConfigLoaderParams { | |||
export interface ConfigLoaderSuccessResult { | |||
resultType: "success"; | |||
configFileAbsolutePath: string; | |||
baseUrl: string; | |||
baseUrl?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is why it's a breaking change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making a previously required property optional is not a breaking change IMO, especially in this case:
- It won't change anything for projects having a
baseUrl
- There's virtually no projects using
tsconfig-paths
that currently do not have abaseUrl
, because it was basically broken
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR introduces a compiler error into any downstream package that happens to use loadConfig
and expects the baseUrl
property to only ever be a string. If a change can cause downstream compiler errors, I consider it breaking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I was not aware of the public API, my bad!
@aleclarson Could you also add an entry for this PR in the CHANGELOG under unreleased heading, noting it as a breaking change. |
6a88d79
to
b1647e4
Compare
LGTM |
Fixes #143
This is a breaking change.