(fix): set rootDir to './src', not './'. deprecate moveTypes #504
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- so there's less silent failures that occur but don't error
- replace overbroad try/catch in getProjectPath with just an
fs.pathExists
- replace overbroad try/catch in cleanDistFolder with just an fs.remove
- fs.remove is like rimraf and `rm -rf` in that it won't error if the
file/dir doesn't exist
- if it does error, it's probably because it *failed* to remove the
dir, and that should error, because it's potentially a problem,
especially if you're publishing right after
- rewrite moveTypes() so it doesn't have an overbroad try/catch
- use fs.pathExists first and early return if it doesn't exist
- only check for known errors with fs.copy, and rethrow others
- this way if copy or remove actually fail, they will actually error
- before they would silently fail, which could similarly be pretty
bad if one were to publish right after a silent failure
- rootDir needed to be changed to ./src because the previous ./ caused
type declarations to be generated in dist/src/ instead of just dist/
- the moveTypes function handled moving the declarations back into
dist/, but occassionally had errors moving .d.ts files
- particularly in CI and for projects with many of them
- declarationMap (*.d.ts.map) files would also have issues due to
the hackiness of moveTypes, setting to rootDir to './src' is one
of the necessary steps in fixing them
- deprecate moveTypes and add a warning with instructions if it is used
when a rootDir of './' is detected
- add notes about a deprecation window in the comments
This was referenced Mar 6, 2020
This was referenced Mar 12, 2020
This was referenced Mar 23, 2020
Open
|
This was not merged after #500 as requested in bold in my opening comment, so its history got squashed into this... See #500 (comment) for why I think |
|
And see also #500 (comment) for a completion to the CI error saga: #691 has a fix to the root cause of the There are still issues with |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
rootDir needed to be changed to ./src because the previous ./ caused
type declarations to be generated in dist/src/ instead of just dist/
dist/, but occassionally had errors moving .d.ts files
the hackiness of moveTypes, setting to rootDir to './src' is one
of the necessary steps in fixing them
deprecate moveTypes and add a warning with instructions if it is used
when a rootDir of './' is detected
Split out from #488 and built on top of #500 (please merge that first) .
fs.copyissues being called inside ofmoveTypesis the root cause of the very frequent CI issues we've had with missing.d.ts.files, which I've occasionally replicated locally (I saw it locally first and then it started happening more in CI).sources#488 goes into more details about the breaking change of changingrootDir, so here I've deprecated with a warning + instructions instead of fully removing it. I've also moved it to adeprecated.tsfile with a clear notice in a comment.sources#488 ((fix): declarationMaps (*.d.ts.map) should have correctsources#488 depends on this fix, so this can go first but not vice-versa. and (fix): declarationMaps (*.d.ts.map) should have correctsources#488 is dependent on Enable useTsconfigDeclarationDir if declarationDir #468 which still hasn't been merged)