Skip to content
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

Support more glob patterns for namespaces #52

Closed
BjoernRave opened this issue Feb 13, 2020 · 11 comments · Fixed by #57
Closed

Support more glob patterns for namespaces #52

BjoernRave opened this issue Feb 13, 2020 · 11 comments · Fixed by #57
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@BjoernRave
Copy link
Contributor

BjoernRave commented Feb 13, 2020

It would be great if I could define my namespaces with patterns like the following:

{
    "*/new": ["forms"],
    "*/[id]/edit": ["forms"]
}
@aralroca aralroca self-assigned this Feb 13, 2020
@aralroca aralroca added the enhancement New feature or request label Feb 13, 2020
@aralroca
Copy link
Owner

I thought about this 🙂Ok, let's do it!

@BjoernRave
Copy link
Contributor Author

@aralroca when you see my config you will know why :D

@aralroca aralroca added this to the 0.8.0 milestone Feb 13, 2020
@aralroca
Copy link
Owner

aralroca commented Feb 14, 2020

@BjoernRave what do you think if instead of glob patterns, we will support regExp when it exist the prefix rgx:?

Ex:

{
 'rgx:^.*/test/.*/example$': ['namespace'] // /this/test/is/an/example -> true
}

In this case, we can provide more flexibility without bringing extra code. The problem of Glob is that we need to do all the conversions or use an external library. In static websites is not a problem because this code is only for the CLI, but in the case of appWithI18n is going to increase the size of that package.

@BjoernRave
Copy link
Contributor Author

BjoernRave commented Feb 14, 2020

@aralroca not sure. I myself am not super fluent with regexes, same for many people I think.

Glob patterns seem to be like a standard for this kind of configuration.

@aralroca aralroca modified the milestones: 0.8.0, 0.9.0 Feb 14, 2020
@Hanzo93
Copy link

Hanzo93 commented Feb 14, 2020

I like the idea of regex support, since its native in javascript it would keep the library tiny

@aralroca
Copy link
Owner

Ok @BjoernRave @Hanzo93 . Let's do a list of pros and cons:

Pros/Cons Globbing Regex
Versatility 👎Good enough, but regex is much richer. 👍 You can do far more complicated things with regular expressions.
Lib size 👎Requires to implement a glob to regex conversor or to import an external lib... So is adding extra kb to the bundle size. 👍Is native in JavaScript, so is possible to implement without writing so much code.
Maintenance 👎 We will need to maintain that extra code of Glob to Regex conversor. 👍TC39 is mantaining the RegExp for us.
Easy to use 👍Easy to learn and remember, is very simple. 👎Hard to learn and sometimes can be frustrating.
Performance 👎 Similar performance than regex but with an additional step to convert Glob to Regex. 👍Very similar to glob, but without the additional step.

Do you want to add any points to the list?

I researched and there are a lot of libraries, like webpack, jest, babel that are using regex in their configurations.

@BjoernRave
Copy link
Contributor Author

@aralroca thanks for that comparison. I guess looking at it, Regexes make more sense. It's fine by me if you guys prefer it. Will be a way to improve my Regex game :D

@aralroca
Copy link
Owner

@BjoernRave can you confirm that with the new 0.9.0 release is working well in your project?

Your example:

{
    "*/new": ["forms"],
    "*/[id]/edit": ["forms"]
}

Should be:

{
    "rgx:/new$": ["forms"],
    "rgx:/\\[id\\]/edit$": ["forms"]
}

or with both rules:

{
    "rgx:(/new|/\\[id\\]/edit)$": ["forms"],
}

@BjoernRave
Copy link
Contributor Author

@aralroca when I install latest I only get 0.8.0

@aralroca
Copy link
Owner

@BjoernRave you are right, I forgot to publish to npm, now is ready https://www.npmjs.com/package/next-translate?activeTab=versions

@BjoernRave
Copy link
Contributor Author

@aralroca can confirm, that it works, good job :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants