-
Notifications
You must be signed in to change notification settings - Fork 33
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: nestjs 10 support fix spelling error #15
fix: nestjs 10 support fix spelling error #15
Conversation
@@ -22,6 +15,14 @@ import { | |||
} from '../constants'; | |||
import { CrudActions } from '../enums'; | |||
|
|||
const { | |||
CUSTOM_ROUTE_AGRS_METADATA = CONSTANTS['CUSTOM_ROUTE_ARGS_METADATA'], |
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.
maybe can change to
CUSTOM_ROUTE_AGRS_METADATA = CONSTANTS['CUSTOM_ROUTE_ARGS_METADATA'] || CONSTANTS['CUSTOM_ROUTE_AGRS_METADATA']
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.
It's destructuring CONSTANTS and if available it takes CUSTOM_ROUTE_AGRS_METADATA, if not it takes the default value which is CONSTANTS['CUSTOM_ROUTE_ARGS_METADATA']
Here a small example of the first case https://www.typescriptlang.org/play?#code/MYewdgzgLgBAwgeQHIGUAqBBJaUwLwwDeAUDPAKroICyA+gEoLloCitGA4vSrdS5gBEMmAFwwA5FACm0ccQC+AbmLFQkWIRikKVOo2ZtO3XvwxDM+eMnRYcAbXFxKaGgyat29Djz6DhGcQBdABoFS0RUTGwUZVVwCBAAGykAOkSQAHMACiddNwN2Lh9TcwwASkUgA
and here of the second https://www.typescriptlang.org/play?#code/MYewdgzgLgBAwgeQHIGUAqBBJaUwLwwDeAUDPAKroICyA+gEoLloCitG9A4irdS5gBEMmAFwwA5FACm0ccQC+AbmLFQkWIRikKVOo2ZsMnejz6DhGfPGTosOANri4lNDQZNW7Lqf4YhmcQBdABoFK0RUTGwUZVVwCBAAGykAOkSQAHMACmdddwN2Yx9zTABKRSA
As you can see in both cases CUSTOM_ROUTE_AGRS_METADATA is defined.
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.
It's destructuring CONSTANTS and if available it takes CUSTOM_ROUTE_AGRS_METADATA, if not it takes the default value which is CONSTANTS['CUSTOM_ROUTE_ARGS_METADATA']
Here a small example of the first case https://www.typescriptlang.org/play?#code/MYewdgzgLgBAwgeQHIGUAqBBJaUwLwwDeAUDPAKroICyA+gEoLloCitGA4vSrdS5gBEMmAFwwA5FACm0ccQC+AbmLFQkWIRikKVOo2ZtO3XvwxDM+eMnRYcAbXFxKaGgyat29Djz6DhGcQBdABoFS0RUTGwUZVVwCBAAGykAOkSQAHMACiddNwN2Lh9TcwwASkUgA and here of the second https://www.typescriptlang.org/play?#code/MYewdgzgLgBAwgeQHIGUAqBBJaUwLwwDeAUDPAKroICyA+gEoLloCitG9A4irdS5gBEMmAFwwA5FACm0ccQC+AbmLFQkWIRikKVOo2ZsMnejz6DhGfPGTosOANri4lNDQZNW7Lqf4YhmcQBdABoFK0RUTGwUZVVwCBAAGykAOkSQAHMACmdddwN2Yx9zTABKRSA
As you can see in both cases CUSTOM_ROUTE_AGRS_METADATA is defined.
thanks @ckaeslin , you are right
Hey, is there anything that prevents you from merging this one? I'm in the middle of upgrading to nestjs v10 and would love this issue to be resolved 🥺 |
@zaro do you have any concerns regarding this PR? |
@ckaeslin I am sorry for answering this late. Not sure why I don't get an email when a PR is opened but only when you @ me . Currently lerna is an issue for running the test suite. I fixed that in master, can you please rebase and I'll get this merged and published after that. |
2d36306
to
c69a25a
Compare
@zaro thanks, I've rebased it. |
Pull Request Test Coverage Report for Build 5507169238Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
@ckaeslin just published 5.3.0 with this PR merged. |
We have used nestjsx crud so far with nestjs 9 and updated to nestjs 10 now.
In nestjs 10 they have removed the misspelled CUSTOM_ROUTE_AGRS_METADATA and only left CUSTOM_ROUTE_AGRS_METADATA nestjs/nest@1a2ac3e which got corrected in nestjs 9.0.6 nestjs/nest@6076545
I've already created an issue on the original library nestjsx#825 but I doubt it gets fixed anytime soon.
Since probably not everyone can or wants to switch to nestjs 10, I've tried to create a solution which works for both cases.
I'm open to any suggestions for a better solution.