Skip to content

(fix): set rootDir to './src', not './'. deprecate moveTypes#504

Merged
jaredpalmer merged 42 commits intojaredpalmer:masterfrom
agilgur5:fix-rootDir-deprecate-moveTypes
Feb 12, 2020
Merged

(fix): set rootDir to './src', not './'. deprecate moveTypes#504
jaredpalmer merged 42 commits intojaredpalmer:masterfrom
agilgur5:fix-rootDir-deprecate-moveTypes

Conversation

@agilgur5
Copy link
Copy Markdown
Collaborator

@agilgur5 agilgur5 commented Feb 11, 2020

  • 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

Split out from #488 and built on top of #500 (please merge that first) .

- 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
@agilgur5 agilgur5 added the version: minor Increment the minor version when merged label Mar 9, 2020
@agilgur5
Copy link
Copy Markdown
Collaborator Author

agilgur5 commented Mar 28, 2020

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 fs.copy was causing issues. The rootDir should still be src, but I think moveTypes might be fixable.

@agilgur5
Copy link
Copy Markdown
Collaborator Author

agilgur5 commented Apr 20, 2020

And see also #500 (comment) for a completion to the CI error saga: #691 has a fix to the root cause of the moveTypes() issue that gave us so many errors and CI issues.

There are still issues with rootDir ./ because it's still not accurate, so changing it to ./src was still correct and does still solve issues (with declarationMaps in particular), but that CI issue now has a root cause fix for it too with #691 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

version: minor Increment the minor version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants