JanCizmar commented 2 years ago

[x] Analyse code and find keys
Make it configurable use React/Angular/Svelte/Vue Tolgee integrations
- [x] React
- [x] Angular
- [x] Vue
- [x] Svelte
[x] Enable user to provide custom parser (could be via some kind od JS plugin or Regex)
[x] Warn user about missing / unused keys in Tolgee platform

Shackless commented 1 year ago

Any news or ETA on this? We are seriously considering tolgee for a large-scale SvelteKit project but we'd need the extractor for Svelte to make it work in our context.

JanCizmar commented 1 year ago

Hey, @cyyynthia might be able to answer.

Or maybe you might be interested into contributing with the Svelte extractor. 😎

cyyynthia commented 1 year ago

Providing an ETA is difficult at this time. That being said, complementing the CLI with missing extractors is a priority. We're currently making polishing touches on the React extractor and the CLI is being experimented in the real world on Tolgee itself, so we can detect CLI bugs and lacking features.

There are no particular priority order for the extractors at this time, but considering Svelte have had the most people requesting it, it might probably be the next extractor that will be worked on. I will make sure to keep this thread updated with what's cooking!

We're also open to contributions, if you're willing to give a helping hand 😉

cyyynthia commented 1 year ago

I have an update regarding the Svelte extractor :eyes:

A good portion of the work required to add Svelte support to the CLI is complete! Remains to do some internal refactoring and actually wire everything up so the CLI makes use of the internal extractor.

You can track progress of the Svelte extractor here: https://github.com/tolgee/tolgee-cli/pull/44

paulwer commented 1 year ago

due to the release of the svelte extractor, can you say, which extractor is planed as next?

cyyynthia commented 1 year ago

Vue is most likely next

paulwer commented 1 year ago

@cyyynthia any updates? :)

cyyynthia commented 1 year ago

Not at this time. I'll keep the thread updated whenever there's something to share 🙂

cyyynthia commented 1 year ago

I've done a little bit of research and here is the plan for implementing support for extracting keys from code. Because Vue is a bit more complicated format, it requires a bit more planning and being aware of Vue's quirks to make an accurate interpretation of the SFC format. I'm not a Vue expert and while I used Vue it was in the early 2.x days, so it's a bit rusty haha. Let me know if I missed a critical detail here.

Only Vue SFC files will be supported. (.vue files). Supporting cases where Vue is used without a bundler is very challenging (tough to identify) and as far as I know represent a very, very niche use case.
Vue SFC files using <script src="./..."/> or <template src="./..."/> will not be supported.
useTranslate calls will be exclusively searched for within setup scripts. It can be the script setup or the setup function exported by the "old" <script>.
$t (and t when applicable) will be looked for in the whole setup script, as well as in the whole <script>. Plus, of course, the template. However, custom blocks will not be looked through.

The order of operations will be the following:

Look through the SFC file and identify the setup script, the other scripts, and the template.
Parse the setup script and identify calls to useTranslate. Only a single one can/will be handled, and it must not be renamed. Keeping the translation function named t is mandatory (and is already a requirement in the other extractors).
- At the same time, also look for calls to this.$t and t within the setup script.
- If not using <script setup>, the setup function must not be a reference to a previously defined method. A warning will be emitted of this is encountered. ^1
- When not using <script setup>, the t function from useTranslate is assumed returned by the setup function.
Parse the rest of the scripts to identify calls to this.$t and this.t.
Parse the template to identify calls to $t or t.

The plan is also to only support HTML templates, at least for the time being. Vue supports using other formats such as pug for its templates, but for now let's stick to the basics and only support "basic" SFCs.

The SFC file being a little bit more complex than a simple linear pass, it will take a bit more time to put together, especially for accurate reporting and ordering of strings.

^1

```js // Good export default { setup () { ... } } // Bad - will emit a warning function mySetup () { ... } export default { setup: mySetup } ```

cyyynthia commented 1 year ago

The initial draft for the implementation is available here: https://github.com/tolgee/tolgee-cli/pull/50

As I mentioned in the PR there, due to complexity of parsing a SFC file and making a correct representation of it, and considering the TextMate grammar needed some tweaks (especially for v-bind and interpolations detection), it'd be much preferable to test the extractor on some real-world examples before moving forward and merging it.

While the extractor has a big test suite to validate it, tests are a highly synthetic kind of workload and doesn't guarantee a real-world SFC file would be correctly understood.

If anyone is willing to test the implementation on a real project using @tolgee/vue, this would significantly help making sure there's no edge-case that has been missed 🙏🏻

paulwer commented 1 year ago

Hey @cyyynthia, i would gladly try to help here. How do I install this into my existing project?

cyyynthia commented 1 year ago

You can try the CLI by cloning it somewhere on your computer, going to the branch where I'm working on the Vue extractor (check on the PR), and follow the instructions in HACKING.md to use run-dev within the CLI project folder.

Then verifying that the output of extract print 'path/to/your/vue/project/**/*.vue is correct. Path can either be absolute or relative to the CLI project folder.

cyyynthia commented 1 year ago

Hey there @paulwer, have you had a chance to give the experimental Vue extractor a look? Feedback would be greatly appreciated and would help moving forward with fixing eventual bugs and releasing it.

paulwer commented 1 year ago

Hey there, first I am sorry, I totaly missed your first response.

I just tried the executor, but without success (maybe wrongly used). My Commands I tried: (on branch: cyyynthia/extractor-vue)

npm run run-dev -- extract print ..\XXXX\frontends\vuexy-vuejs-admin-template\typescript-version\full-version\src\**\*.vue 
npm run run-dev -- extract print ..\XXXX\frontends\vuexy-vuejs-admin-template\typescript-version\full-version\src\**\*
npm run run-dev -- extract print ..\XXXX\frontends\vuexy-vuejs-admin-template\typescript-version\full-version\src
npm run run-dev -- extract print ..\XXXX\frontends\vuexy-vuejs-admin-template\typescript-version\full-version\src\
npm run run-dev -- extract print ..\XXXX\frontends\vuexy-vuejs-admin-template\typescript-version\full-version\src\*
npm run run-dev -- extract print C:\dev\repositories\XXXX\frontends\vuexy-vuejs-admin-template\typescript-version\full-version\src\**\*.vue
npm run run-dev -- extract print C:\dev\repositories\XXXX\frontends\vuexy-vuejs-admin-template\typescript-version\full-version\src\**\*
npm run run-dev -- extract print C:\dev\repositories\XXXX\frontends\vuexy-vuejs-admin-template\typescript-version\full-version\src
npm run run-dev -- extract print C:\dev\repositories\XXXX\frontends\vuexy-vuejs-admin-template\typescript-version\full-version\src\
npm run run-dev -- extract print C:\dev\repositories\XXXX\frontends\vuexy-vuejs-admin-template\typescript-version\full-version\src\*

Reponse in all cases:

(node:30176) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
🐭✅     Analyzing code...
Total unique keys found: 0
Total warnings: 0

What am I doing wrong?

Some more informations about my project: I've been using the following implementations within the project: The VueJs "T" component:

<T :key-name="test" :default-value="test" />

The tolge $t method within .vue files and .ts files.

Also I use both sometimes based on conditional logic or by generated key-names.

I use vite in combination with: "unplugin-auto-import": "^0.15.1", "unplugin-vue-components": "^0.24.1",

cyyynthia commented 1 year ago

Hm, this auto-import plugin is making me wonder: do you auto-import @tolgee/vue? The extractor verifies if the Tolgee SDK is used before proceeding to the extraction to save on performance; if you auto-import the SDK, it likely is failing to detect it and doesn't look through the files.

A workaround since the check is quite naive is to add as a comment "@tolgee/vue" which will make the extractor believe the SDK is imported and make it check.

With that being said, if you're using $t the fact it's failing to extract at least these is concerning. Can you provide just a single file, where $t is being used?

paulwer commented 1 year ago

I've tried your suggestion without success:

Example File from project

<script setup lang="ts">  
// @tolgee/vue  
import { useAuth } from '@/plugins/keycloak'  
import LiveMachineList from '@/views/liveMachine/list/LiveMachineList.vue'  

const auth = useAuth()  

// 👉 VIEW  
const lsName_view = 'liveMachine.list.view'  
type viewType = 'admin' | 'user' | 'organization' | 'manufacturer'  

const views: ComputedRef<viewType[]> = computed(() => [  
  ...auth.currentUserApiOrganization.value ? ['organization'] as viewType[] : [] as viewType[],  
  'user',  
  ...auth.currentUserApiOrganization.value?.isManufacturer ? ['manufacturer'] as viewType[] : [] as viewType[],  
  ...auth.hasResourceRole('manage-machines', 'backend') || auth.hasResourceRole('manage-live-machines', 'backend') ? ['admin']  
 as viewType[] : [] as viewType[],  
])   

const view = ref<typeof views.value[number]>('user')  

watchEffect(() => view.value = views.value.find(i => i === localStorage.getItem(lsName_view)) ?  
 localStorage.getItem(lsName_view) as typeof views.value[number] : views.value[0])  

watchImmediate(view, value => value ? localStorage.setItem(lsName_view, value) : localStorage.removeItem(lsName_view))  
</script>  

<template>  
  <VRow>  
    <VCol  
      cols="12"  
      md="7"   
    >  
      <h4 class="text-h4 mb-6">  
        LiveMachine List  
      </h4>  
      Live Machines are assets, which are registered within the system. More Texts...  
      <br>  
      Currently your view as filtered by machines, which are owned by your organization.   
      <br>  
      You can also choose to display your currently personal assigned machines, machines within your seller group, machines which are connected to you as a customer or an admin-view.  
    </VCol>  

    <VCol  
      cols="12"  
      md="5"  
    >  
      <VSelect  
        v-if="views.length > 1"  
        v-model="view"  
        :label="$t('View')"  
        :items="views.map(t => ({ value: t, title: $t(`liveMachine.list.view.${t}`) }))"  
      />  
      <h5  
        v-else  
        class="text-right"  
      >  
        <T :key-name="`liveMachine.listView.view.${views[0]}`" />  
      </h5>  
    </VCol>  

    <!-- TODO: show right component based on view-parameter -->  

    <VCol cols="12">  
      <!-- 👉 LiveMachine List  -->  
      <LiveMachineList :view="view" />  
    </VCol>  
  </VRow>  
</template>

paulwer commented 1 year ago

is there a way to see how many files were checked, to verify first, the sdk checks them?

paulwer commented 1 year ago

Found the issue: the problem was using \ instead of / within the path on windows.

maybe you should add something like this error statement / issue a warning message for the user OR display how many files were processed at the end-result:

paulwer commented 1 year ago

I now did get a valid result back: The following has drawn my attention: 1 warning was emitted during extraction: line 50: Dynamic key

How does tolgee cli handle these occurences? Could them be evaluated by the typescript type? f.ex.

const lsName_view = 'liveMachine.list.view'  
type viewType = 'admin' | 'user' | 'organization' | 'manufacturer'  

const views: ComputedRef<viewType[]> = computed(() => [  
  ...auth.currentUserApiOrganization.value ? ['organization'] as viewType[] : [] as viewType[],  
  'user',  
  ...auth.currentUserApiOrganization.value?.isManufacturer ? ['manufacturer'] as viewType[] : [] as viewType[],  
  ...auth.hasResourceRole('manage-machines', 'backend') || auth.hasResourceRole('manage-live-machines', 'backend') ? ['admin']  
 as viewType[] : [] as viewType[],  
])   

const view = ref<typeof views.value[number]>('user')  

$t(`liveMachine.list.view.${t}`)

this implementation has used ${t} with type of viewType. therefore 4 possibilities are available here: liveMachine.list.view.admin | liveMachine.list.view.user | liveMachine.list.view.organization | liveMachine.list.view.manufacturer

paulwer commented 1 year ago

Hm, this auto-import plugin is making me wonder: do you auto-import @tolgee/vue? The extractor verifies if the Tolgee SDK is used before proceeding to the extraction to save on performance; if you auto-import the SDK, it likely is failing to detect it and doesn't look through the files.

A workaround since the check is quite naive is to add as a comment "@tolgee/vue" which will make the extractor believe the SDK is imported and make it check.

With that being said, if you're using $t the fact it's failing to extract at least these is concerning. Can you provide just a single file, where $t is being used?

not required, works fine.

cyyynthia commented 1 year ago

Found the issue: the problem was using \ instead of / within the path on windows.

After looking at your commands, it might be due to not wrapping the path in an explicit string (which is recommended in the docs). While using backlashes are not recommended, it should be working just fine for most cases otherwise.

maybe you should add something like this error statement / issue a warning message for the user OR display how many files were processed at the end-result:

There definitely are places where logging needs to be improve and this is a great suggestion. I'll make sure to add this at some point.

How does tolgee cli handle these occurences? Could them be evaluated by the typescript type?

Unfortunately no. The CLI is nowhere near smart enough to gather, understand and use type information. The CLI simply turns the files into a stream of semantic tokens and consumes them using a state machine with limited context retention. This makes for relatively good performance and accurate interpretation of the file, but doesn't allow to know at any given point things like available variables in scope (information not retained), let alone their type.

Whenever there is a warning, you have a few possibilities:

Refactor your code to use a switch statement. This is what Tolgee itself does.
Use the magic comments @tolgee-key or @tolgee-ignore to manually provide information about the key to the CLI.

A dynamic key warning (or any warning besides dynamic default string) will make it skip the key (so no invalid key makes it way into your Tolgee project). They will also make commands such as sync abort unless --continue-on-warning is specified, as a safety mechanism.

not required, works fine.

If you use $t everywhere T is used, yes the CLI will flag all the relevant files for processing by the extractor without the need to import @tolgee/vue (or make the CLI believe it was imported).

The fact it's working and producing valid results is an encouraging sign! Now, to validate the parsing is doing its job as expected, can you confirm that

There are no missing keys from the extraction, besides cases where warnings have been raised?
There are no false positives or seemingly invalid keys getting extracted?

paulwer commented 1 year ago

from my pov: .ts files are not analysed, even when i do **/* or **/*.* instead of **/*.vue anything I can do?

cyyynthia commented 1 year ago

The Vue extractor only considers .vue files for extraction, as it expects everything to be within SFCs. What is the use-case for checking .ts (and by extension .js) files as well?

paulwer commented 1 year ago

within vue development there are several use-cases to use them: From my pov the biggest are the plugins like vue-router (routing / guards) and vuex/pinia (state-management).

Or in the simplest case the main-entrypoint of the application (main.js /.ts) (which defines the plugin and env. configuration at load and mount the main App Component (ex. App.vue)).

Or just to defined .ts-files for shared functionality.
=> in my case useMachineTranslation() to be used in multiple *.vue Components with the case statement you described above as best practise.

cyyynthia commented 1 year ago

Hm, I see, it actually makes a lot of sense now that I think about it haha.

Just to confirm since I'm not a Vue expert, in all these scenarios importing from @tolgee/vue is mandatory in order to call useTranslate, right? I'm assuming there is no access to the global injected $t in plain ts files

paulwer commented 1 year ago

I guess we have to look at 2 topics here:

Auto Imports => https://github.com/antfu/unplugin-auto-import
Component Imports => https://github.com/antfu/unplugin-vue-components

=> i suggest reading the readme.md of each project, to get a better understanding, what they do.

Auto Imports

According to the https://github.com/antfu/unplugin-auto-import plugin documentation, tolgee or any library can be imported automaticly, when defined so.

This is a vite related topic, not vue in general. To reduce complexity I would suggest to put this section within the documentation and make the import a requirement, if you dont see any easy workaround here. Or display a warning, when tolgee/vue is found in the vite plugin configuration as auto-import. (the dev has to define it as auto-imported)

As vite compiles the programm code, it adds the imports afterwards. (from my understanding, this can be plain ts, vue, svelte or react [any library compiled with vite])

This auto import feature can be used in any files, with typescript in it. therefore in .vue and in .ts files in the case of my project.

Component Imports

This instead is vue related, and enables to import components for the template sections of a vue component, without importing them in the file. (the T component for example)

I will test this next week, but from my POV the parser already detects the T component as it is defined as global component.

Possible solution

Maybe the vue extractor can build the programmcode first, therefore all imports are placed valid (when i understand it right) Too complex or edge-case related?

Example of my vite-configuration: vite.config.ts.zip

cyyynthia commented 1 year ago

I'll do a global answer instead of a point-by-point answer.

The CLI doesn't operate at a project-level but on a per-file level. It doesn't have any knowledge of a vite config, a webpack config, or any tooling config for that matter. This means, it only sees what has been explicitly defined in the files, and cannot "bundle" them in any capacity since it doesn't know how to do so (and there's an infinite amount of ways a project might be bundled).

This means, any form of automatic import ("magic globals") will cause problems. To decide whether a file should be processed or skipped, the CLI does a preliminary check to see if the file imports the SDK (or uses certain known globals such as $t for Vue SFC files). It it doesn't, the file is skipped. The SDK import check is also used to detect which extractor to use: attempting to extract uses of the React SDK with the Svelte extractor would not work. Again, this is done at file-level since the CLI doesn't have a notion of "project".

For Vue SFC files, the fact it detects the T component without importing it is likely because you're also using $t within the file which flags the file for extraction. It doesn't do a detailed verification to ensure the T component was indeed imported from the SDK and isn't a component with the same name. Same for useTranslate.

It should also be noted the CLI is written to be language agnostic. It uses a generic parsing technique using TextMate grammar files, and that's about it. It doesn't have any JavaScript-specific processing done besides that, by design. This is meant to make the CLI able to be extended to non-web languages in the future.

The long story short here is that any plugin that does add imports at build-time will cause problems, especially with tools which operate on the file-level with extremely limited global insight like the CLI.

My personal opinion is that they're a bad practice and comes with huge hidden costs that greatly outweighs the eventual QoL improvements (making code harder to read with a lot of "globals", unclear intents, unclear import trees, tooling issues and conflicts, ...), but my personal opinion isn't the subject here.

In the future it might be possible to try solving these issues but they'll inevitably come with the cost of specifying even more things manually to the CLI, instead of having a clean code file that is readable as-is by anyone, human or machine, with clear indications of what it's doing and using - making the CLI able to know what you're doing.

Coming back to my question, it was more targeted at use of translations within non-component files (reusable composables, route declarations, etc). Without the case of plugins adding them manually, in these scenarios using plain .ts files, one has to import the SDK one way or another, right?

paulwer commented 1 year ago

Yes, you are right. In those cases the import is necessary.

cyyynthia commented 1 year ago

Alright then; knowing that Vue SFC appear to be extracted properly I'll be able to add extraction of .ts files using useTranslate without too much troubles and move forward with the extractor.

Besides the auto-imports which can't be solved at this time (although stuffing a @tolgee/vue comment in files not detected can be a good workaround), once this is done I think we'll be able to move forward with bringing Vue support to the CLI officially. Watch #50 for updates 👍🏻

paulwer commented 1 year ago

maybe an additional vue3 behavior: @tolgee/vue provided t function as Ref. this means, within setup functions or external files (.ts-files) the function has to be invoked with .value.

I've send you 2 example case-statements, which are related to your react case statement implementation:

Please check of this would be catched by the extractor:

Or the vue-lib to be changes to dont use a ref in this case. (but I am not so much into it, which impact changing this could have, within an existing project)

=> I looked it up, most likely due to onNsUpdate, this would not be a viable solution. But feel free to check it yourself.

import type { Components as DefaultApiComponents } from "@/plugins/apis/default";
import { useTranslate } from "@tolgee/vue";
import type { viewType } from "./list/store";

export function useLiveMachineTranslation() {
  const { t } = useTranslate();

  return {
    view: (input: viewType) => {
      switch (input) {
        case "admin":
          return t.value("liveMachine.list.view.admin");
        case "manufacturer":
          return t.value("liveMachine.list.view.manufacturer");
        case "organization":
          return t.value("liveMachine.list.view.organization");
        case "user":
          return t.value("liveMachine.list.view.user");
        default:
          throw new Error("invalid input");
      }
    },
  };
}

This should be catched already from my pov:

import type { Components as DefaultApiComponents } from "@/plugins/apis/default";
import { useTranslate } from "@tolgee/vue";
import type { viewType } from "./list/store";

export function useLiveMachineTranslation() {
  const t = useTranslate().t.value;

  return {
    view: (input: viewType) => {
      switch (input) {
        case "admin":
          return t("liveMachine.list.view.admin");
        case "manufacturer":
          return t("liveMachine.list.view.manufacturer");
        case "organization":
          return t("liveMachine.list.view.organization");
        case "user":
          return t("liveMachine.list.view.user");
        default:
          throw new Error("invalid input");
      }
    },
  };
}

cyyynthia commented 1 year ago

Thanks for pointing that out. I actually think I got it wrong including in some cases in SFCs themselves since I assume .value is unnecessary all the time, which is only the case within <template>.

paulwer commented 1 year ago

it is also possible to use the $t as follows:

const anykey = useTolgee().t.value;
const { value as t } = useTolgee().t;
const { t } = useTolgee();
const anykey = useTranslation().value.t;
const { t as anykey } = useTranslation().value;

does the extractor reconise this patterns? I would considder it not as best practise to assign a custom keyname, but the cli should look for it => users may unpredictable :) If not i guess issueing a warning for these patterns could be easier, than finding, thes dynamic keynames in the code afterwards? If the extractor is able to find them, this would be of cause the best solution.

OR maybe you should start to create a best-practise guide for the integrations.

cyyynthia commented 1 year ago

The CLI is quite picky with that and has quite a few pitfalls it does not detect nor issue warnings for. Generally, the general assumption is that people will use the CLI on sane code and don't do exotic things in their code.

Namely, the CLI will fail short if:

useTranslate (or equivalent) is renamed during import (or otherwise "renamed" by assigning it to a new variable)
t (or equivalent) is renamed in any capacity (like in the examples you've shown)
Users deviate from framework-specified best practices (or framework-intended way of consuming things)

The last point covers attempts to access the .value key directly at the ref computation site: Vue does not intent refs to be immediately unwrapped

As for the other ways to access t, while they work and are documented within the API reference on the Tolgee documentation website, the "Translating" section doesn't mention it and I consider accessing them this way more of a hack than a "proper" access. @stepan662 might be able to provide more input on this question as the dev of the SDK

To be honest, when drafting the extractors initially, I considered supporting weird edge cases (and do for some), but I consider the inability of the CLI to extract weird cases like these ✨not a bug but a feature✨: they help in some sense enforcing best practices and strongly encourage the use of stable high-level APIs rather than certain lower-level APIs that are meant for other use cases. Emitting warnings for these can be good, but there's another problem with those and that's once again the limited insight of the CLI.

Tracking symbol names etc requires some degree of runtime-level understanding of the code, and being able to precisely track how functions and objects will be used at runtime, which is extremely difficult for static analysis tools to do, especially in highly dynamic languages like JavaScript which can allow extremely exotic code. Tools like TypeScript have this degree of understanding, but is extremely more complex and would become a burden to maintain in the CLI (in addition to be very language-specific).

For the reference, in the prototypes I'm working on at vite-plugin-i18n-tolgee (not pushed yet), renaming symbols or assigning them to new variables is tightly controlled and can quickly raise errors and refuse to compile. It also is extremely picky on how you should use the symbols from the plugin and very quickly errors out. It is capable of doing so because:

It has access to a much more detailed Abstract Syntax Tree that represents the code (like eslint and TypeScript does) while the CLI only has a stream of semantic tokens
It actively watches the code during build allowing a strict control of how things are used immediately, unlike the CLI which only passively looks through already written code
It can make very language-specific assumptions and draw much better conclusions, whereas the CLI should stay relatively language agnostic for the ease of maintenance and extensibility

While the CLI is much more limited in its understanding, it could be possible to make it detect certain things (so long they're within a same file; cross-file interactions are impossible to account for with the current architecture), at the expense of making machines that are already quite long and complex even longer and more complex. It might also be possible to do some clever design and use a separate service-machine to detect and issue warnings for those, with the help of the parent machine giving it information about what to track and what not. It'd be the subject or a separate issues altogether though imho.

I hope this long explanation wasn't too boring and answered your questions :D

stepan662 commented 1 year ago

I think we might solve this by making the parser a bit more simple, rather than more complex. For example, I think it would be quite ok, to just assume t function is called t and not check what is the output of useTranslate - so to decouple this a bit. For me it make sense to have the namespace extraction "optional" - if it fails, just assume the namespace empty.

paulwer commented 1 year ago

I agree simple is sometimes the best solution. And as already mentioned good and accessable documentation should also be key. (f.ex. displayed as a comment after/while execution or linked best practises).

from py POV the following cases should be supported:

t()
$t()
.value syntax
T component
warnings for dynamic keys
file analysis, when @tolgee/* is detected

What do you think about this?

cyyynthia commented 1 year ago

The problem is that it's not that simple to extract things in a generic way. Most extractors have some degree of language and framework specific understanding and syntax that is not possible to account for in a generic fashion - nor I think they should be done in a generic way

t function works relatively the same for all, so much so the extraction has already been generalized for JavaScript and most machines don't handle this and delegate to a submachine.

For everything else there's some degree of variability and subtle differences needing handling on a case-by-case. This is why there's not a "single" extractor but 3 at the time and it's not possible to write a "do it all" one because framework-specific handling cannot be done in a generic way and requires the CLI to know what it's dealing with (this is why it relies on the SDK import and/or additional clues to know what to look for)

Additionally, even if it was possible, .value, and $t are imo specific properties of each framework and should be treated as such and handled on a per-case basis. For Svelte, $t is from getTranslate (different from other frameworks useTranslate) while Vue's is global and cannot have a namespace tied to it (so we can't share the logic of the Svelte extractor, otherwise it'd try to look for a hook/composable call - or might confuse them and apply namespaces when it shouldn't).

On not complaining when the useTranslate call is not found and extract t anyway, I'd disagree with going this route. I feel like this is a source of problems for the end-user, and I made the extraction picky with the intent to make the CLI trustable: no need to spend a long time reviewing and adjusting all it did - invalid data is something that must not happen. Missed keys may happen, and the goal is to reduce these instances with warnings as much as possible.

A blank namespace instead of a specific namespace doesn't sound like a big deal to a human mind because we're able to make some nuances, but for software it's radically different. It can't have the deep understanding we have and be like "oh, I saw this key and it just needs to be moved!", to the CLI its a brand new key with all the consequences it has (deleting the old key, not forwarding old comments, etc.). There's ways to make it easier on the user by asking them about it (key exist in another namespace, etc...) but we just moved the complexity to the command handler making sense of the extracted data and forward it onto the user.

With all that being said, I think emitting a warning when t (and/or whatever relevant) is found but no hook/composable was found, can be a good compromise to balance all the different needs at play. Same for renames, I think it's possible to write generic-ish logic for that and emit warnings. This way we don't loose the quality of data, but we also sorta "extract" keys we'd currently miss ; except they're still not extracted but we warn the user about this possiblity and encourage a manual review on their end with file and line info.

stepan662 commented 1 year ago

Warnings are a viable alternative, I guess, but it will add complexity imho.

As @paulwer said, more simple is sometimes better. I don't think we can cover all the usecases the user can come up with, but if we'll have some simple rules for the extraction, so as a developer I'll know what I should do in order to make it work.

JanCizmar commented 1 year ago

simple is sometimes better

🚀🚀🚀🚀🚀🚀🚀🚀

cyyynthia commented 1 year ago

I'd agree but the truth is that trying to have something simpler in terms of the method used is actually more difficult to achieve with good results here! As I said, there's a lot of room for confusion internally and cases that will result in poor extraction and hacky workarounds to make the extraction work as intended (the biggest problem being retaining the namespace from the hook/composable when applicable (and only when applicable!!)).

Instead, I think my approach is actually simpler than what's proposed if you see it as a divide-and-conquer algorithm. I don't see the extraction procedure as a single problem, I see it as smaller sub-problems with no interaction:

Extracting the components is a single problem
Extracting $t global is a single problem
Extracting t is a single problem
...

With this logic, it makes it much easier to think about the problems and know what these individual components should behave like. I don't have to worry about namespace retention polluting the handling of global $t or T, or other problems specific to a single sub-problem affecting the larger picture.

Detecting t and others can be considered a single sub-problem, because it is significantly smaller in size (if we see something named t or relevant, job is done). It does add complexity to the global program, but I think it is isolated enough it is easier to manage compared to a solution that appears simple on paper but introduces dependencies that makes the problem harder to divide in isolated blobs.

paulwer commented 10 months ago

@cyyynthia the following is not detected: Am I missing something?😇

Its a .vue file and the screenshot is from the setup script

cyyynthia commented 10 months ago

I think it doesn't like t being renamed to $t in the script setup section

paulwer commented 10 months ago

Both cases does not work, only not working in the setup scipt of .vue files. In normal ts files its working great.

the script is wrapped in this: <script setup lang="ts"> ... </script>

I also tried with these two: const $t = useTranslate().t.value; const $t = useTolgee().value.t;

paulwer commented 10 months ago

Also maybe it could be a good idea to have a cli option to tag unused keys, so they can be viewed in the admin-ui more conveniant and the user can descide, which of them should be delted

Command like: --unused-tagging

The cli should also remove existing tags, when they are used again in the next analyse.

cyyynthia commented 10 months ago

Both cases does not work, only not working in the setup scipt of .vue files.

Looking at the test code, it seems I wrote it such that it expects value not to be unwrapped: https://github.com/tolgee/tolgee-cli/blob/main/test/unit/extractor/vue/extract.test.ts#L321-L352

I highly suggest to open a dedicated issue for this, as well as the feature request for tagging unused keys to not clutter this thread and keep things organized. 🙂

stepan662 commented 1 month ago

108 Implements angular extractor

paulwer commented 2 weeks ago

Hi folks, we want to use tolgee within our backend using typescript to determine some notification subjects. Therefore we would like to have a parser for .ts files. Is this already possible?

JanCizmar commented 2 weeks ago

I believe, you can add your custom extractor. Would this solve your issue? https://tolgee.io/tolgee-cli/extraction/custom-extractor

tolgee / tolgee-cli

Create configurable command to find keys in the code #2

Example File from project

Auto Imports

Component Imports

Possible solution

Please check of this would be catched by the extractor:

This should be catched already from my pov:

108 Implements angular extractor