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: nestjs 10 support fix spelling error #15

Merged

Conversation

ckaeslin
Copy link

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.

@@ -22,6 +15,14 @@ import {
} from '../constants';
import { CrudActions } from '../enums';

const {
CUSTOM_ROUTE_AGRS_METADATA = CONSTANTS['CUSTOM_ROUTE_ARGS_METADATA'],

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']

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@denciu
Copy link

denciu commented Jul 3, 2023

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 🥺

@ckaeslin
Copy link
Author

@zaro do you have any concerns regarding this PR?

@zaro
Copy link
Member

zaro commented Jul 10, 2023

@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.

@ckaeslin ckaeslin force-pushed the fix/nestjs10-support-fix-spelling-error branch from 2d36306 to c69a25a Compare July 10, 2023 10:48
@ckaeslin
Copy link
Author

@zaro thanks, I've rebased it.

@coveralls
Copy link

coveralls commented Jul 10, 2023

Pull Request Test Coverage Report for Build 5507169238

Warning: 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

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.05%) to 99.075%

Totals Coverage Status
Change from base Build 5506798780: -0.05%
Covered Lines: 1247
Relevant Lines: 1248

💛 - Coveralls

@zaro zaro merged commit c452059 into gid-oss:master Jul 10, 2023
2 checks passed
@zaro
Copy link
Member

zaro commented Jul 10, 2023

@ckaeslin just published 5.3.0 with this PR merged.

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.

None yet

6 participants