Fixes rubenv/angular-gettext-tools#177

[matthieusieben]

Allow using markerNames to detect string literals. Simply add a leading comment to the string that contains the markerName (exactly) and that string will be added to the pot file.

Example:

const myString = /* gettext */ "This will be extracted"

or

var extractor = new Extractor({ markerNames: [ "@ngTranslate" ] })

Will extract the following string:

const myString = /* @ngTranslate */ "This will be extracted"

[rubenv]

Hi, thanks for writing this PR, but I wish you'd have checked with me first.

A big part of the design philosophy in angular-gettext was simplicity: one way to do things. Somewhat as a counter-reaction to angular-translate where there's a thousand options and equally much ways for doing things. Users get lost and hate it.

As such I'd rather not have two ways of doing the same thing, it'll just lead to confusion.

[matthieusieben]

The problem with your way is that it is utterly inefficient as it requires a useless function call in production code... Plus, it does not integrate properly with some code bundlers which sometimes rename functions (e.g. Rollup).

[matthieusieben]

This is the problem with the NPM community... You want to improve stuff, but you can't because the first guy who proposed an "ok" solution is now ruling on his own little kingdom without listening to its peers for improvements.

The alternative is for me to fork this project, meaning that there will be 2 ways of doing things out there (on NPM), defeating your purpose... There will then be 2 distinct code base which won't benefit from each other's fixes, leading to a decreased user experience for the community.

All because you wanted to impose your way. This is so absurd...

[rubenv]

Woah chill mate. There's a reason I didn't close this one yet: there's always room for talking about things.

The rollup case is pretty interesting and I hadn't considered that one. Though I do wonder if it's a good idea to extract strings from minified (as opposed to original) code. Is there no way to hook into this process before minification?

[rubenv]

Similarly, it's probably possible to add a build step (after extraction) that removes the calls to gettext(), as they are no-ops anyway.

But again, let's try to understand each other first before bursting into a fit of rage. I'm pretty sure we can come up with a solution that works for everybody.

[matthieusieben]

Rollup works as follows (roughly):

1. builds a tree of dependencies
2. applies transforms to individual files
3. resolves global naming conflicts (if some modules declares a gettext variable, even in a protected scope, the following global variables called gettext will be renamed into gettext$1...n)
4. merges all the files into one bundle

It also has a live watch functionality which only applies step 2 to the updated files and then applies step 3 and 4.

The most efficient way to apply the extractor.extractJs() is during step 2, as only updated files need to be parsed again. This would allow both to extract the gettext() strings, and remove the useless function call (btw, extractor.extractJs would need an option to transform the code, removing noops calls, to avoid having to re-build the AST in the rollup plugin).

However, doing this a a major inconvenient: with the live watch functionality on, new strings would only be added to the extractor instance, and, outdated strings, never be cleaned. I have run into several memory leak issues due to this.

Because of this memory leak issue, the proper way to integrate with the current version of angular-gettext-tools is to use the "on bundle hook" to parse the code (before minification of course).

Now the problem with this hook, is that it comes after the step 3) described before, during which RollupJs renames variables that may be in conflict, resulting in some gettext() function calls being renamed into gettext$1().

The way I see it, there are several solutions to this issue:

• Change the behaviour of RollupJS. This won't work since they are renaming variables in order to generate safe code, even when use strict is not used.
• Find a way to link strings to the files they come from in the Extractor so that they can be cleaned when they are removed from files during live watch. This would require a major re-write of this lib.
• Find a way to reliably detect translatable strings. I believe that one decent requirement is to impose the detection to be done before minification (which is required to detect /// comments). I don't think it is safe or efficient to rely on the function name though.

IMO the best solution is to always rely on special comments (which are never tempered with by code transformers), and to deprecate every other way of detecting strings.

[rubenv]

Note that we also extract strings from calls to gettextCatalog.getString() and gettextCatalog.getPlural(), so it's not purely gettext() that needs to be taken into account. This last one has more than one string in the definition.

[matthieusieben]

I know. I thought of it and have to admit that Javascript does not provide enough tools to handle this case.

gettextCatalog.getPlural(/* @gettextn */ "One", "Several")
gettextCatalog.getPlural(/* @gettext */ "One", /* @plural */ "Several")

Are all ugly!

[matthieusieben]

Another solution would be:

gettextCatalog.getPlural(/* @gettext */ [ "One", "Several" ])

But it would require an update of angular-gettext, which is probably not a good idea.

[matthieusieben]

Or one could write:

gettextCatalog.getPlural.apply(gettextCatalog, /* @gettext */ [ "One", "Several" ])

But my propositions are getting worse and worse 😆

[rubenv]

Yeah, that's not exactly ideal 😁

[matthieusieben]

I think that from the language point of view, the best would probably be:

// Singular only string
/* @gettext */ "My string"
/* @gettext */ [ "My string" ]

// String with plural
/* @gettext */ [ "One", "Some" ]

// String with multiple plural forms
/* @gettext */ [ "One", "Some", "A lot" ]

The array notation has the advantage of allowing to support defining more than one plural form, as some languages require, and is not currently supported by angular-gettext

[rubenv]

The array notation has the advantage of allowing to support defining more than one plural form, as some languages require.

Base language should always be English (if you do it right), so that's probably not a problem here :-)

Also keep in mind that we can't break compatibility. There are some pretty large codebases using this, they're not going to be happy if everything needs to be changed.

[ngehlert]

@matthieusieben is this issue still relevant? I still don't quite understand your problem. There is no reason that the extraction has to be after minification or transform (on a sidenote, you still can configure this step to force variable/function names to remain the same, e.g. ngInject does sth similar for angularjs services)
even if this would be for some reason not possible you can set up a separate job only to extract the strings while developing and trigger you other watch to minify again.

Can you try to explain your use case a little bit more if this is still relevant?