Skip to content
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: remove deprecated decorator passing in tsconfig path hooks #2023

Closed
wants to merge 1 commit into from

Conversation

ttshivers
Copy link

@ttshivers ttshivers commented Apr 9, 2023

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

nest-cli passes decorators to updateImportDeclaration which has been deprecated since typescript v4.8 now that decorators are placed on modifiers in the syntax tree.
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-8.html#decorators-are-placed-on-modifiers-on-typescripts-syntax-trees

nest-cli fails to build with typescript 5 with errors like https://github.com/nestjs/nest-cli/pull/1986/checks?check_run_id=12600352096

> @nestjs/[email protected] build
> tsc

lib/compiler/hooks/tsconfig-paths.hook.ts:58:26 - error TS2339: Property 'decorators' does not exist on type 'ImportDeclaration'.

58                     node.decorators,
                            ~~~~~~~~~~

lib/compiler/hooks/tsconfig-paths.hook.ts:62:21 - error TS2554: Expected 5 arguments, but got 6.

62                     node.assertClause,
                       ~~~~~~~~~~~~~~~~~

lib/compiler/hooks/tsconfig-paths.hook.ts:78:26 - error TS2339: Property 'decorators' does not exist on type 'ExportDeclaration'.

78                     node.decorators,
                            ~~~~~~~~~~

lib/compiler/hooks/tsconfig-paths.hook.ts:83:21 - error TS2554: Expected 6 arguments, but got 7.

83                     node.assertClause,
                       ~~~~~~~~~~~~~~~~~


Found 4 errors in the same file, starting at: lib/compiler/hooks/tsconfig-paths.hook.ts:58


Exited with code exit status 2

Issue Number: N/A

What is the new behavior?

nest-cli no longer emits deprecation warnings and also now successfully builds with typescript 5

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

Decorators are now placed on modifiers on TypeScript's syntax trees.
Passing decorators separately has been deprecated since v4.8 and removed completely in v5.

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-8.html#decorators-are-placed-on-modifiers-on-typescripts-syntax-trees
microsoft/TypeScript#49089

@ttshivers ttshivers force-pushed the fix-typescript-5 branch 2 times, most recently from 4ad6f34 to 521dc06 Compare April 15, 2023 19:54
@ttshivers ttshivers changed the title Update and fix compatibility with typescript 5 fix: remove deprecated decorator passing tsconfig path hooks Apr 15, 2023
@ttshivers
Copy link
Author

I've removed the commit to update typescript to v5 in this commit and just addressed the deprecated decorator passing instead for this PR.

@ttshivers ttshivers changed the title fix: remove deprecated decorator passing tsconfig path hooks fix: remove deprecated decorator passing in tsconfig path hooks Apr 15, 2023
Decorators are now placed on `modifiers` on TypeScript's syntax trees.
Passing decorators separately has been deprecated since v4.8 and removed
completely in v5.

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-8.html#decorators-are-placed-on-modifiers-on-typescripts-syntax-trees
microsoft/TypeScript#49089
@kamilmysliwiec
Copy link
Member

Let's track this here #2092

@aashir-khan
Copy link

@kamilmysliwiec did that PR address the deprecation? I seems like it addressed something else

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants