-
-
Notifications
You must be signed in to change notification settings - Fork 11
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
Create configurable command to find keys in the code #2
Comments
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. |
Hey, @cyyynthia might be able to answer. Or maybe you might be interested into contributing with the Svelte extractor. 😎 |
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 😉 |
I have an update regarding the Svelte extractor 👀 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: #44 |
due to the release of the svelte extractor, can you say, which extractor is planed as next? |
Vue is most likely next |
@cyyynthia any updates? :) |
Not at this time. I'll keep the thread updated whenever there's something to share 🙂 |
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.
The order of operations will be the following:
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// Good
export default {
setup () {
...
}
}
// Bad - will emit a warning
function mySetup () { ... }
export default { setup: mySetup } |
The initial draft for the implementation is available here: #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 |
Hey @cyyynthia, |
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 Then verifying that the output of |
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. |
Hey there, I just tried the executor, but without success (maybe wrongly used).
Reponse in all cases:
What am I doing wrong? Some more informations about my project:
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: |
Hm, this auto-import plugin is making me wonder: do you auto-import A workaround since the check is quite naive is to add as a comment " With that being said, if you're using |
I've tried your suggestion without success: Example File from project
|
This comment was marked as outdated.
This comment was marked as outdated.
I now did get a valid result back: How does tolgee cli handle these occurences? Could them be evaluated by the typescript type?
this implementation has used ${t} with type of viewType. therefore 4 possibilities are available here: |
not required, works fine. |
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.
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.
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:
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
If you use 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
|
from my pov: .ts files are not analysed, even when i do |
The Vue extractor only considers |
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. |
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 |
I guess we have to look at 2 topics here:
=> i suggest reading the readme.md of each project, to get a better understanding, what they do. Auto ImportsAccording 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 ImportsThis 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 solutionMaybe the vue extractor can build the programmcode first, therefore all imports are placed valid (when i understand it right) Example of my vite-configuration: |
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 For Vue SFC files, the fact it detects the 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 |
Yes, you are right. In those cases the import is necessary. |
Alright then; knowing that Vue SFC appear to be extracted properly I'll be able to add extraction of Besides the auto-imports which can't be solved at this time (although stuffing a |
maybe an additional vue3 behavior: 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.
This should be catched already from my pov:
|
Thanks for pointing that out. I actually think I got it wrong including in some cases in SFCs themselves since I assume |
it is also possible to use the $t as follows:
does the extractor reconise this patterns? OR maybe you should start to create a best-practise guide for the integrations. |
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:
The last point covers attempts to access the As for the other ways to access 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:
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 |
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 |
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:
What do you think about this? |
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
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, On not complaining when the 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 |
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. |
🚀🚀🚀🚀🚀🚀🚀🚀 |
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:
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 Detecting |
@cyyynthia the following is not detected: Its a .vue file and the screenshot is from the setup script |
I think it doesn't like |
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 optional:tagname The cli should also remove existing tags, when they are used again in the next analyse. |
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. 🙂 |
The text was updated successfully, but these errors were encountered: