ngx-translate-extract
ngx-translate-extract copied to clipboard
Are you going to take over "biesbjerg/ngx-translate-extract-marker" too?
Thank you for taking over the library from biesbjerg.
He also made a second library, which provides a marker to directly extract translations from a component by decorating it with a marker.
--> https://github.com/biesbjerg/ngx-translate-extract-marker
This library did not receive any updates in three years too, seems to be working fine, but is still based on View Engine. This results in a warning when installing it:
Processing legacy "View Engine" libraries:
- @biesbjerg/ngx-translate-extract-marker [es2015/esm2015] (https://github.com/biesbjerg/ngx-translate-extract-marker.git)
Are there any plans to take over this library too?
@StefanKern I do not wish to impose on or steal Vendure's thunder in taking over the biesbjerg's great work, but given a sudden need to upgrade both extract and extract-marker at our company I've recently forked and updated both of these libraries to support Ivy (thanks to already existing PRs) as well as some other features such as marker pipe and directive. So if you're in a rush you can look up those in the meantime while this issue is being resolved; extract, marker.
All of these features is also included in my PR to vendure's repo, here #5.
Thanks @colsen1991 will be using your version until the situation is clear
Hi chaps, I'm really sorry - I had failed to set up notifications for this repo 🤦 so I wasn't aware of the activity here. A dropped ball for sure given the circumstances of this fork!
In any case, now I'm making my way through the PRs and will issue a new version today.
OK v8.0.5 is out now.
And this got me thinking - why is the marker function a separate package anyway? Is it because one is a dev dependency and the other is a regular dependency?
It seems an awful lot of overhead for what is essentially an identity function. What's a better way to do this? Open to suggestions here but in general my bias is to reduce dependencies wherever possible. Better for the consumers and maintainers!
I think it was splitted to accomodate project dependencies because marker in meant to be included into final user projects while the extract is just tool required in "dev" requirement only. (that way you can avoid downloading the extract tool and all of its dependencies when you build you app in a automated ci for example)
I proposed a merge request to @colsen1991 's fork (not sure that it is available in the pull request here though ?) that re-added an old way of using the marker (that was in this project before but was removed at some point but my project was dependent on it and it was easier to re insert the feature rather than migrate my project, basically you can define your own custom function as a marker so that you do not have to import the one from the other repo and you just specify the name of the custom marker function on the command line with the --markeror -m option, also imho it is more versatile that the current method, because you can do what you want in the custom function))
@michaelbromley I don't know the history of the marker lib, but @tmijieux brings up a good point regarding tree shaking and the two libs being very different in terms of dev and runtime dependency. So i suggest the two libs stay separate. Also given that not everyone who uses extract needs the marker fn as extract also works with translate fn, pipe and directive.
So I suggest that either:
- @colsen1991/ngx-translate-extract-marker lives on (BTW, soon to be @husbanken/ngx-translate-extract-marker) as a standalone lib and I update my PR to redirect to it instead of @biesbjerg's marker lib
- or that you (Vendure) fork my fork of marker fn with its improvements and take over future maintanance
From what I can tell marker is just a no-op and I don't see many changes being introduced there in the future, especially since my fork now supports custom marker fn with the --marker and -m option. and a marker directive and pipe.
Either way I will update my PR to this repo to include the other improvents introduced in my fork, mark my fork as deprecated, redirect to yours (@vendure/ngx-translate-extract) and discontinue publishing it. Although I'll leave the published versions up on NPM for anyone who's added it as a dependency.
What do you guys think?
Hi @colsen1991
Long-term, I guess it makes sense for the extract & marker libs to both live in the Vendure org. Right now I don't want to hold up your work on your PR so I'd say we can leave it pointing to the husbanken version until I have a spare moment to fork and set up a new npm package.
I'm also interested in at some point exploring the proposal of @tmijieux about an API for a custom extract function.
I'm also interested in at some point exploring the proposal of @tmijieux about an API for a custom extract function.
you can try it out at npm @colsen1991/ngx-translate-extract (or @aum_biosync/ngx-translate-extract)
old commit from Biesbjerg that originally introduced the feature: https://github.com/Husbanken/ngx-translate-extract/commit/bc5ce7e80d67192105260568b97618926558779f https://github.com/Husbanken/ngx-translate-extract/commit/daaebede6f78d724ed66228d4d33b7e30444547b
commit that removed it: https://github.com/Husbanken/ngx-translate-extract/commit/4fe3c436244335fcb7ade90786eecf001314c7db
commit where i re-introduce it: https://github.com/Husbanken/ngx-translate-extract/commit/f8d1279c1e84de24a49926fa08043627a1d7e970
note that the custom marker was originally replaced by "regular marker" and in the new commit, both feature are available and the -m option is a basically switch between both feature (only one at a time... but that could be changed as well if one want both simultaneously)
@michaelbromley yeah, makes sense for them to coexist under the same namespace, so let's talk after I've updated my PR (I'll let you know once its ready) and it has been merged and published.
And as @tmijieux mentioned the custom marker function has already been re-implemented in my fork, so if you want to test it out just follow his instructions. I'll look into including this option in the PR if you want?
@michaelbromley just updated the PR. Sorry for the mess, but I hoep you'll be able to parse/power through it :)
Also as mentioned there, I'm gonna prep a branch on my fork of ngx-translate-extract-marker to make it easier for you to take over that lib as well.
@michaelbromley branch for marker lib should be ready: https://github.com/Husbanken/ngx-translate-extract-marker/tree/chore/move-to-vendure-ecommerce :)
@colsen1991 I see you created the fork but the librarary "@vendure/ngx-translate-extract-marker" is not released yet, isn't it?
Hi all! I'm currently in a bit of a crunch to get Vendure v2 released next week. After that I'll find some time to address this issue.
No worries, @michaelbromley! And no, it's not published yet - I only created the branch to accomodate Vendure taking over :) So it should just be fork, merge, code review and publish on your end 👍
Can I somehow help you with it?
@colsen1991 thanks for prepping things like that, makes my life much easier :)
@StefanKern my ideal setup would not be to have 2 separate git repos, but to have the marker package also published from this repo, making this a monorepo to keep everything together. If that is something you are interested in helping with, let me know and I'll make you a maintainer so you can work on that independently of me. If not, I will just form Christer's repo for now and publish from there.
ps Servus aus Wien!
So you would like to merge the code into a single repository and publish both libraries from here? I would like to take a look at it in the next days and maybe I can be of help :)
PS: Grüße aus Wien zurück :)
So you would like to merge the code into a single repository and publish both libraries from here?
Yes exactly. Although the actual publishing step, I can take care of. Take a look and see if that's something you think you can do. If you need anything from me just let me know. For a start you should be fine just cloning this repo, but if you later want write access give me a shout :)
Hey, I thought this would be a quick change, but then also did not really have the time for it. Also I'm no longer convinced it would be a good idea. ngx-translate-extract and ngx-translate-extract-marker user very different technologies. Merging them together then creates a pretty large package.json and makes the generation of the two packages more complex.
So after trying it out for way to long, I would keep the two projects.
With the recent release of ngx-translate@16, the core library now includes a built-in marker function: Migration Guide - Using the ngx-translate-_-marker Function.
I suggest updating the documentation to point to this direction. For simple use cases, the built-in marker function can be highlighted as the preferred approach. For more advanced scenarios where a marker pipe or directive is required, the documentation could still reference the implementation by @colsen1991.