Feature/multiple loaders and cli #547

vsamofal · 2023-08-21T17:28:53Z

Description

I implemented 3 features, one small, and two that actually broke existing functionality.

I added the ability to parse a response formatted, so everyone will be able to customize it if needed.
I implemented a cli to generate types and watch for generation, pointing to multiple folders. (this one doesn't support custom loaders yet, but it's possible to implement it as well if someone needs it). Now only json and yaml supported
also, I added the ability to specify multiple loaders. for me it is useful with mono repo, or when some library provides default translations, so I can load it. My specific use case is to package validation errors and distribute them across the organization
also fixed all eslint and prettier issues

I also need to update the documentation if these changes are ok to do, also it's a breaking change probably, version 11 makes more sense.

I don't expect this pr to be merged, but more to have a conversation about all of these changes.

Thanks

Linked Issues

Additional context

the easiest way to test a client is to

npm install @saas-buildkit/nestjs-i18n
nestjs-i18n -h

added ability to pass an optional custom response body formatter

Feature/custom response body

changed generated file default path

fixed tests that become broken accidently

fixed most current tests

made fixes to make it work properly removed watch mode because I don't like even idea to generate it in runtime

… properly

…uces a breaking changes

…ders Feature/support multiple loaders

…ders renamed base methods for validations to make code cleaner

…ders updated version

improvements for tests and imports did a lot of manual testing for the cli with different situations for json

…on the fly anymore

…s_client Feature/proper generate types client

…s_client updated version and added changing permissions for cli file after build

… and prevent names duplications

rubiin

as long as no tests break fine with me. For the cli feature, @toonvanstrijp will have to give his confirmation

src/filters/i18n-validation-exception.filter.ts

…s_and_cli # Conflicts: # package.json # samples/dto-validation/src/app.module.ts # samples/simple/src/app.controller.ts # samples/simple/src/app.module.ts # src/i18n.constants.ts # src/i18n.module.ts # src/loaders/i18n.abstract-file.loader.ts # src/loaders/i18n.json.loader.ts # src/loaders/i18n.loader.ts # src/loaders/i18n.yaml.loader.ts # src/resolvers/header.resolver.ts # src/resolvers/query.resolver.ts # src/services/i18n.service.ts # src/utils/merge.ts # src/utils/typescript.ts # tsconfig.json

…conds just for safety and play with debounce

…th github workflow

# Conflicts: # tests/i18n-fastify.e2e.spec.ts

coveralls · 2023-08-22T15:37:36Z

coverage: 87.259% (-3.6%) from 90.828%
when pulling 38dffc3 on saas-buildkit:feature/multiple_loaders_and_cli
into 2eb5f4e on toonvanstrijp:main.

vsamofal · 2023-08-22T16:56:32Z

I did fix all issues with pipelines.

@toonvanstrijp let me know when you can take a look and approve the work, so I can do the documentation part, and because of the breaking change it probably will be 11.0.0?

toonvanstrijp

@vsamofal awesome work so far! I'm really happy and grateful for the effort you're putting in. I think with a few more things to discuss we could make an awesome V11 release that's going to take this library in the right direction!

src/filters/i18n-validation-exception.filter.ts

toonvanstrijp · 2023-08-22T16:46:09Z

src/cli.ts

@@ -0,0 +1,17 @@
+#!/usr/bin/env node


@vsamofal I'm a very big fan of this feature! One of the early versions of nestjs-i18n had this feature implemented. But I think we could even improve on this a little bit more.

What if we move the I18nOptions to a file (e.g. i18n.ts) just like how mikro-orm does it too. That way we make the i18n-cli use that config, but also let the nestjs module use that config.

This would end up with a better workflow and configuration.

toonvanstrijp · 2023-08-22T16:54:28Z

src/loaders/i18n.abstract-file.loader.ts

-        this.events.pipe(switchMap(() => this.parseTranslations())),
-      );
-    }
+  async load(): Promise<I18nTranslation> {


I think we might need to be careful with removing the Observable as a return type. If we change it to a promise instead, it won't be possible to have translation be loaded in real time or be pushed be some other event. This means you would need to call refresh yourself whenever is needed. What are your thoughts on this @rubiin ?

I agree. Promise and Observables work differently. Observables are mostly preferred way of working with streams i.e series of data unlike promises which would mostly resolve a single value and be done

toonvanstrijp · 2023-08-22T16:59:38Z

tests/i18n-express.e2e.spec.ts

-          loaderOptions: {
-            path: path.join(__dirname, '/i18n/'),
-          },
+          loaders: [


Is it still possible to use dependency injection in the loader? Now if you loader for example used your mikro-orm connection it won't be possible to inject this. Maybe we need to rethink the loader options a bit. Since if we're going to make breaking changes for a V11, we can make a few more to change this we don't particularly like now.

…lder and removed auto generation

# Conflicts: # package-lock.json # package.json # src/i18n.context.ts # src/i18n.module.ts # src/index.ts # src/loaders/i18n.abstract-file.loader.ts # src/loaders/i18n.json.loader.ts # src/loaders/i18n.yaml.loader.ts # src/services/i18n.service.ts

vsamofal · 2023-10-04T20:20:09Z

@rubiin @toonvanstrijp Hi guys, when you have time can you take a look

rubiin · 2023-10-05T04:39:24Z

Will be cherry picking some commits that are no breaking

thiagotognoli · 2023-10-06T00:07:11Z

@rubiin @toonvanstrijp Hi guys, when you have time can you take a look

please approve this pr, we need these changes

B0ER · 2023-10-13T16:52:01Z

package.json

  "main": "./dist/index.js",
  "types": "./dist/index.d.ts",
  "files": [
    "dist"
  ],
  "scripts": {
    "prepare": "npm run build",
+    "postbuild": "chmod +x ./dist/cli.js",


why do we need to set execution mode?

B0ER · 2023-10-13T17:04:28Z

src/commands/generate-types.command.ts

+  return newPath.startsWith('./') ? newPath.slice(2) : newPath;
+}
+
+function sanitizePaths(paths: string[]) {


does it work for win system?

B0ER · 2023-10-13T17:04:44Z

src/commands/generate-types.command.ts

+
+function sanitizePath(path: string) {
+  // adding trailing slash
+  const newPath = path.endsWith('/') ? path : `${path}/`;


can we use node:path.parse function?

B0ER · 2023-10-13T17:11:49Z

src/filters/i18n-validation-exception.filter.ts

  catch(exception: I18nValidationException, host: ArgumentsHost) {
    const i18n = I18nContext.current();

    const errors = formatI18nErrors(exception.errors ?? [], i18n.service, {
      lang: i18n.lang,
    });

+    const normalizedErrors = this.normalizeValidationErrors(errors);
+
    switch (host.getType() as string) {
      case 'http':


should we process the 'rpc' context type?

B0ER · 2023-10-13T17:30:31Z

src/utils/import.ts

@@ -0,0 +1,80 @@
+import fs from "fs"
+import path from "path"
+import {pathToFileURL} from "url"


here it's not formatted, but
I'm not sure that it was needed to re-format all to another code-style, definitely imports, order of import and private\public methods
(if it's prettier fix - ok)

B0ER · 2023-10-13T17:42:07Z

tsconfig.json

@@ -6,11 +6,14 @@
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "skipLibCheck": true,
+    "importHelpers": false,


why you turn off importHelpers?

# Conflicts: # package-lock.json # samples/dto-validation/pnpm-lock.yaml # samples/simple/pnpm-lock.yaml # src/filters/i18n-validation-exception.filter.ts # src/i18n.context.ts # src/i18n.module.ts # tests/app/controllers/hello.controller.ts # tests/app/examples/example.functions.ts # tsconfig.json

rubiin · 2024-04-22T11:09:35Z

Lets break the changes into different PR . Would be easier to review and rollout considering the breaking changes.

rubiin · 2024-04-24T18:03:06Z

closed in favor of #628

vsamofal added 30 commits August 13, 2023 21:55

added ability to pass an optional custom response body formatter

68800cf

Merge pull request #1 from saas-buildkit/feature/custom-response-body

ec0eb1d

added ability to pass an optional custom response body formatter

renamed repo for local publishing

925f899

added host argument to the i18n filter response builder

f9a8adf

Merge pull request #2 from saas-buildkit/feature/custom-response-body

aaef498

Feature/custom response body

changed generated file default path

de1c5d1

Merge pull request #3 from saas-buildkit/fix/changed_import_path

54580ac

changed generated file default path

fixed tests that become broken accidently

079caff

Merge pull request #4 from saas-buildkit/fix/changed_import_path

6c84a20

fixed tests that become broken accidently

added base support for multiple loaders

6389af2

fixed most current tests

fixed remaining tests

02edd85

added simple tests for multiple folders

f78e4e0

made fixes to make it work properly removed watch mode because I don't like even idea to generate it in runtime

added test that checking the override and adding more keys is working…

966ad6b

… properly

updated version for release, incrementing to 11.0.0 because it introd…

1a304fc

…uces a breaking changes

fixed integration tests with app

b9f4351

added some sample keywords

d1e3d99

Merge pull request #5 from saas-buildkit/feature/support_multiple_loa…

0e0b47c

…ders Feature/support multiple loaders

renamed base methods for validations to make code cleaner

754b72b

Merge pull request #6 from saas-buildkit/feature/support_multiple_loa…

0cac3bf

…ders renamed base methods for validations to make code cleaner

updated version

950678e

Merge pull request #7 from saas-buildkit/feature/support_multiple_loa…

3342b04

…ders updated version

added implementation for proper type generation client

4f00bfe

fixes for build process

9b429d3

improvements for tests and imports did a lot of manual testing for the cli with different situations for json

fixed lint rules and excluded dist from linting

7f1c037

get rid of observables logic because we don't need to generate types …

46d70b2

…on the fly anymore

updated version

e0a5375

Merge pull request #8 from saas-buildkit/feature/proper_generate_type…

8c6f25e

…s_client Feature/proper generate types client

updated version and added changing permissions for cli file after build

19ada5a

Merge pull request #9 from saas-buildkit/feature/proper_generate_type…

15df7bc

…s_client updated version and added changing permissions for cli file after build

rolled back base functions name to simplify aliasing on a client side…

5e38b1a

… and prevent names duplications

rubiin reviewed Aug 21, 2023

View reviewed changes

src/filters/i18n-validation-exception.filter.ts Show resolved Hide resolved

vsamofal added 6 commits August 21, 2023 19:45

fixed after merge conflicts resolutions

aef8c57

fixed keywords after merge

847df79

updated waiting for 500 ms instead of 2 seconds in tests, it was 2 se…

2ad190e

…conds just for safety and play with debounce

fixed tests to stop watching after test finished to prevent issues wi…

cdceacd

…th github workflow

Merge branch 'main_base' into feature/multiple_loaders_and_cli

a407944

# Conflicts: # tests/i18n-fastify.e2e.spec.ts

vsamofal added 2 commits August 22, 2023 17:18

improved test coverage including failed scenarios

e87235f

added not testable files to ignore

745f9c0

toonvanstrijp requested changes Aug 22, 2023

View reviewed changes

rubiin requested a review from toonvanstrijp August 25, 2023 06:55

vsamofal added 6 commits September 22, 2023 16:54

feature: added options file provider to simplify flow of using

aa1e08f

fix: proper build for the cli

1caed04

fix: improvements for importing files in tests

e0983ea

fix: made package.json test more stable by moving it to a separate fo…

b30799a

…lder and removed auto generation

chore: fixes after resolving conflicts

e58766f

rubiin mentioned this pull request Oct 5, 2023

WIP: v10.4.0 #579

Merged

B0ER reviewed Oct 13, 2023

View reviewed changes

Repository owner deleted a comment from B0ER Apr 22, 2024

rubiin closed this Apr 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/multiple loaders and cli #547

Feature/multiple loaders and cli #547

vsamofal commented Aug 21, 2023 •

edited

Loading

rubiin left a comment •

edited

Loading

coveralls commented Aug 22, 2023 •

edited

Loading

vsamofal commented Aug 22, 2023

toonvanstrijp left a comment

toonvanstrijp Aug 22, 2023

toonvanstrijp Aug 22, 2023

rubiin Aug 24, 2023

toonvanstrijp Aug 22, 2023

vsamofal commented Oct 4, 2023

rubiin commented Oct 5, 2023

thiagotognoli commented Oct 6, 2023

B0ER Oct 13, 2023

B0ER Oct 13, 2023

B0ER Oct 13, 2023

B0ER Oct 13, 2023

B0ER Oct 13, 2023 •

edited

Loading

B0ER Oct 13, 2023

rubiin commented Apr 22, 2024

rubiin commented Apr 24, 2024

Feature/multiple loaders and cli #547

Feature/multiple loaders and cli #547

Conversation

vsamofal commented Aug 21, 2023 • edited Loading

Description

Linked Issues

Additional context

rubiin left a comment • edited Loading

Choose a reason for hiding this comment

coveralls commented Aug 22, 2023 • edited Loading

vsamofal commented Aug 22, 2023

toonvanstrijp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vsamofal commented Oct 4, 2023

rubiin commented Oct 5, 2023

thiagotognoli commented Oct 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

B0ER Oct 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rubiin commented Apr 22, 2024

rubiin commented Apr 24, 2024

vsamofal commented Aug 21, 2023 •

edited

Loading

rubiin left a comment •

edited

Loading

coveralls commented Aug 22, 2023 •

edited

Loading

B0ER Oct 13, 2023 •

edited

Loading