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

Use lazy loading import for i18n files #2

Merged
merged 2 commits into from
Feb 6, 2020
Merged

Conversation

jweisman
Copy link
Contributor

@jweisman jweisman commented Feb 5, 2020

Intended to resolve the following issues:

  • 404 errors for non-existent languages/translations from TranslateHttpLoader
  • JSON files are cached after new build and translations are not available

Solution based on ngx-translate/http-loader#25 (comment)

@jweisman jweisman requested a review from exlbashirn February 5, 2020 12:32
@jweisman jweisman self-assigned this Feb 5, 2020
@jweisman
Copy link
Contributor Author

jweisman commented Feb 5, 2020

@exlbashirn - can you check out this solution to the caching issue (and the 404s)? I'd be interested if there is a better option to the relative load of the files (i.e. ../../.ng/...). Thanks!

@exlbashirn
Copy link
Collaborator

Hi @jweisman

I'm not sure about the dependency importing on behalf the dependent.

There is not a lot of room here to do it differently with dynamic imports since webpack needs the hint, so if you want to do it this way and have it be transparent, then what you did is probably it, but this is making a lot of assumptions... though I suppose in our case they're likely to be true most of the time.

The alternative with the same approach is to instead have the import outside in the app.

Also, if you're doing it this way, it would not make sense to have the translations in the assets folder, as they would no longer be needed as static files.


export { MaterialModule, getTranslateModule };
export { MaterialModule, getTranslateModule, LazyTranslateLoader };
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the export really needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I moved the files out of assets and into /src/i18n. And you're right- no need for the export here.

I looked for alternatives to the import in the dependency, but I couldn't find anything better. The assumptions are definitely more brittle than I would like. But it may be the best option at this point.

@exlbashirn exlbashirn merged commit 5aa3370 into master Feb 6, 2020
@jweisman jweisman deleted the ngx-cache-busting branch February 6, 2020 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants