feat: initial support for case type selection #2208

espoal · 2023-07-31T15:27:50Z

Summary

This is the second of a multi PR effort aiming at supporting custom name casing conventions, as detailed here.

The first PR attempts to update @nestjs/schematics to accept a caseType option when generating a new file.
In this PR we add a caseNaming field to GenerateOptions
This is the PR to document the new option
Future PRs will extend this feature in term of supported actions, tests and configuration

For now I decided to support only controllers, to reduce the changes size and make reviewing easier. If the approach will be approved I will make sure to extend it to other objects and to add more tests.

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] 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?

Currently @nestjs/cli uses kebab-or-snake case, as mentioned in this issue.

Issue Number: 462

What is the new behavior?

The goal is to add a caseNaming option to nest-cli.json so that generated files will follow the desired name casing convention.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other informations

How to run

Install deps and build:

npm install
npm run build

Build updated @nestjs/schematics
Clone this branch, and do:

npm install
npm run build

Install the updated @nestjs/schematics package
Copy the dist folder in @nestjs/nest-cli deps: nest-cli/node_modules/@nestjs/schematics/dist/
Run it with one of those commands:
node bin/nest.js new test-nest -s --caseNaming=camel to generate a custom nest-cli.json
node bin/nest.js g co keb-pap in a folder with the correct nest-cli.json

lib/schematics/schematic.option.ts

espoal · 2023-08-15T13:46:34Z

Hey @micalevisk can I help you with anything? I would love a pair programming session to review the pr together and see how to improve it

mwanago · 2023-08-15T19:55:43Z

@espoal
I think it would make sense to add the --caseNaming option to the new command that creates a new NestJS application. Would you agree?

micalevisk · 2023-08-15T20:07:37Z

yeah, instead of adding that new option to generate command, we can move it to new and to the configuration file (nest-cli.json)

mwanago · 2023-08-15T20:18:50Z

@micalevisk
Since we usually want to keep the same naming convention throughout the project, storing this option in nest-cli.json sounds like a great idea

espoal · 2023-08-17T06:17:20Z

@mwanago Let's split the problem in two parts:

My main concern is if my approach of decorating SchemaOption is a good idea, or if maybe people see a better approach.
I would totally be ok to use the new command. Could you make an example of how a CLI command would be using that? And where the nest-cli.json is parsed?

micalevisk · 2023-08-27T19:17:04Z

@espoal

I would totally be ok to use the new command. Could you make an example of how a CLI command would be using that?

it would be nothing more than a new option (--caseNaming <option>) for the new command that will define the value of the caseNaming option (defaults to "kebab") of the generated nest-cli.json (I guess at GenerateOptions interface here: https://github.com/nestjs/nest-cli/blob/48939b57814f0353e1759fcabb85fe401b39e793/lib/configuration/configuration.ts#L76C18-L76C33)

Then, when using the generate command, that option will be taken into account just like it does for the spec option which uses the value from nest-cli.json.

In this case, we won't have --caseNaming for others commands.

espoal · 2023-09-29T13:18:15Z

Sorry for the late reply but I was swamped with my day work.
The PR is far from perfect, but I think I'm slowly getting closer to what you meant @micalevisk .
Some questions:

I still cannot change the suffix of the file (KebPap.controller.ts instead of KebPapController.ts). Any advice on how to proceed?
Now I correctly use the nest-cli.json but are we sure we don't want to leave the command line option? Now we have both.
I cannot see why the pipeline is failing. Locally it seems to work. Suggestions on how to fix it?

And a slightly bigger topic:

Now when we use the new command the file don't respect the passed convention, because they're taken from a template. The solution IMHO would be to update the new command to use a sequence of commands (configuration, generate ....) instead of the templates. But this seems like a big topic, so maybe you have suggestions.

cc: @mwanago

package.json

espoal · 2023-10-05T12:33:57Z

if we can make it work without adding the options to the generate command, then yes. Less code to maintain; less options to know about. We can add them in the future if needed.

I think it's not too much code, but I also think that smaller PRs are better. Let's keep the flag for a future PR.

The project generated by new is too simple.

That's a fair point

Also, convert this PR to draft until you're done.

I still don't think the PR is perfect, but perfection is also the worst enemy of the good, so I would like to ask you @micalevisk to give another look.

Things I would like to do after this review:

Decide which case formats we support
Add documentation
Add more tests

P.S.: I managed to fix the pipeline 🥳

test/lib/utils/formatting.spec.ts

actions/generate.action.ts

espoal · 2023-10-17T07:29:43Z

test/lib/schematics/schematic.option.spec.ts

    {
      description: 'should manage classified string option value name',
      option: 'name',
      input: 'MyApp',
      expected: 'my-app',
    },
-    {
+    /*{
      description: 'should manage parenthesis string option value name',


Atm we don't really respect this. In package.json we have the right name my-(app), but at least on linux the folder is actually called ./'my-(app)' (note the dashes). Same for the [ character.
The test fails because the case conversion function performs sanitization, but I could fix it with this:

case 'kebab': return kebabCase(str, { keep: ['[', ']', '(', ')'] })

espoal · 2023-10-17T07:30:25Z

test/lib/schematics/schematic.option.spec.ts

@@ -38,19 +38,19 @@ describe('Schematic Option', () => {
      input: 'myApp',
      expected: 'my-app',
    },
-    {
+    /*{
      description: 'should allow underscore string option value name',


I feel this could be a source of confusion: if I specify the kebab case, I would expect the convention to be respected.
The code atm reflect this behavior, hence the test fails because my_app turns to my-app.

espoal · 2023-10-17T07:38:09Z

In the end I decided that the case we should support are:

snake
kebab
camel
pascal

I would propose to drop the kebab-or-snake case for the reasons outlined here. Let me know if you would prefer to keep it for now and remove it in a future PR.

@micalevisk I would love to ask you for another review

espoal · 2023-11-06T16:21:29Z

Hey @micalevisk is there anything I could do to speed up the merging of this PR?

micalevisk · 2023-11-06T16:24:03Z

@espoal nop. I'll try to review this ASAP

espoal · 2023-11-06T16:30:12Z

Thanks, I apologies to bother you but I'm really excited about my first nestjs PR ❤️

micalevisk · 2023-11-13T23:55:11Z

lib/utils/formatting.ts

+  }
+};
+
+export const formatString = (str: string) => {


can you elaborate here in the code on what this function is supposed to do?

github copilot can help :p

Dang, I'm surprised copilot understood immediately the code. I'm starting to question reality :D

Added some JsDoc. Basically this function escapes parenthesis which will then later be removed.

I see here an opportunity to create @nestjs/utils or @nestjs/stringUtils because this code is shared across projects, and I feel it would be beneficial to centralize it so that it's easier to maintain and can be reused by plugin developers. I didn't do it to keep the scope limited, but it shouldn't be too hard to do and I would be up for it.

ok I updated the same code also in @nestjs/schematics

espoal added 3 commits July 31, 2023 17:20

feat: case naming

4e4668e

chore: linting and formatting

f0065ed

feat: make schematic option config optional

bfb061a

espoal changed the title ~~feat: initial support for case naming~~ feat: initial support for case type selection Jul 31, 2023

espoal mentioned this pull request Jul 31, 2023

feat: initial support for case type selection nestjs/schematics#1501

Open

3 tasks

feat: update jsdoc

d2cc288

espoal commented Jul 31, 2023

View reviewed changes

lib/schematics/schematic.option.ts Outdated Show resolved Hide resolved

espoal mentioned this pull request Jul 31, 2023

Add configuration option to generate files with PascalCase instead of the default naming convention #462

Closed

1 task

espoal added 3 commits August 3, 2023 15:26

chore: fixing typos

3b015b2

fix: consistent option naming

40c5032

feat: improved switch case loop

9235d0d

espoal marked this pull request as ready for review August 15, 2023 13:45

espoal added 10 commits September 27, 2023 14:25

chore: package.json versioning

0eef7e9

feat: nest cli

33301f4

feat: read case naming from cli

b59ae2e

chore: cleanup

7592150

chore: more cleanup

5cba7cf

chore: more cleanup

14190bd

chore: more cleanup

3d80b1b

chore: more cleanup

fa607e3

chore: more cleanup

cf1ddf7

chore: more cleanup

50fe8b2

micalevisk reviewed Sep 29, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

espoal added 2 commits October 4, 2023 18:46

Merge branch 'master' into feat/caseNaming

ee1f782

chore: removed cli option

707dbb1

milsosa reviewed Oct 5, 2023

View reviewed changes

test/lib/utils/formatting.spec.ts Outdated Show resolved Hide resolved

actions/generate.action.ts Outdated Show resolved Hide resolved

espoal added 2 commits October 5, 2023 14:55

chore: fixed npm version for case-anything

5b0afbb

feat: added tests for strings

7c15944

espoal marked this pull request as ready for review October 16, 2023 08:05

espoal added 6 commits October 16, 2023 14:11

feat: snake case

8cd3da7

Merge branch 'master' into feat/caseNaming

2eaf507

chore: align package-lock.json

9abfe52

chore: aligning package-lock.json

1ac1093

feat: remove kebab-or-snake case

a482f79

chore: silenced some tests

b4048db

espoal mentioned this pull request Oct 17, 2023

feat: document case naming generate option nestjs/docs.nestjs.com#2886

Open

3 tasks

espoal requested a review from micalevisk October 17, 2023 07:22

espoal commented Oct 17, 2023

View reviewed changes

feat: sanitze kebab-or-snake input

b730e41

espoal mentioned this pull request Oct 17, 2023

[Minor-Security] String formatting function is prone to misuse / abuse #2336

Closed

4 tasks

espoal added 3 commits October 18, 2023 12:06

Merge branch 'master' into feat/caseNaming

e5d57c2

feat: fix semantic version of case-anything

301cf0c

feat: explicit mention of kebab-or-snake case

3531cd2

micalevisk reviewed Nov 13, 2023

View reviewed changes

espoal added 2 commits November 15, 2023 08:44

feat: better docs

46d1f48

Merge branch 'master' into feat/caseNaming

5a5618b

espoal requested a review from micalevisk November 15, 2023 08:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: initial support for case type selection #2208

feat: initial support for case type selection #2208

espoal commented Jul 31, 2023 •

edited

espoal commented Aug 15, 2023

mwanago commented Aug 15, 2023 •

edited

micalevisk commented Aug 15, 2023 •

edited

mwanago commented Aug 15, 2023

espoal commented Aug 17, 2023

micalevisk commented Aug 27, 2023 •

edited

espoal commented Sep 29, 2023 •

edited

espoal commented Oct 5, 2023 •

edited

espoal Oct 17, 2023 •

edited

espoal Oct 17, 2023 •

edited

espoal commented Oct 17, 2023 •

edited

espoal commented Nov 6, 2023

micalevisk commented Nov 6, 2023

espoal commented Nov 6, 2023

micalevisk Nov 13, 2023

espoal Nov 15, 2023 •

edited

espoal Nov 15, 2023

feat: initial support for case type selection #2208

Are you sure you want to change the base?

feat: initial support for case type selection #2208

Conversation

espoal commented Jul 31, 2023 • edited

Summary

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other informations

How to run

espoal commented Aug 15, 2023

mwanago commented Aug 15, 2023 • edited

micalevisk commented Aug 15, 2023 • edited

mwanago commented Aug 15, 2023

espoal commented Aug 17, 2023

micalevisk commented Aug 27, 2023 • edited

espoal commented Sep 29, 2023 • edited

espoal commented Oct 5, 2023 • edited

espoal Oct 17, 2023 • edited

Choose a reason for hiding this comment

espoal Oct 17, 2023 • edited

Choose a reason for hiding this comment

espoal commented Oct 17, 2023 • edited

espoal commented Nov 6, 2023

micalevisk commented Nov 6, 2023

espoal commented Nov 6, 2023

micalevisk Nov 13, 2023

Choose a reason for hiding this comment

espoal Nov 15, 2023 • edited

Choose a reason for hiding this comment

espoal Nov 15, 2023

Choose a reason for hiding this comment

espoal commented Jul 31, 2023 •

edited

mwanago commented Aug 15, 2023 •

edited

micalevisk commented Aug 15, 2023 •

edited

micalevisk commented Aug 27, 2023 •

edited

espoal commented Sep 29, 2023 •

edited

espoal commented Oct 5, 2023 •

edited

espoal Oct 17, 2023 •

edited

espoal Oct 17, 2023 •

edited

espoal commented Oct 17, 2023 •

edited

espoal Nov 15, 2023 •

edited