Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Use webpack alias to resolve the languages file #2730

Merged
merged 4 commits into from
Mar 1, 2019

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Mar 1, 2019

Hopefully this will end up simpler than having to figure out in riot-web what the relative path is from react-sdk's src/languageHandler.js to riot-web's webapp directory.

Requires element-hq/element-web#9014

Hopefully this will end up simpler than having to figure out in
riot-web what the relative path is from react-sdk's
src/languageHandler.js to riot-web's webapp directory.
// LANGUAGES_FILE is a webpack compile-time define, see webpack config
const url = (typeof LANGUAGES_FILE === "string") ? require(LANGUAGES_FILE) : (i18nFolder + 'languages.json');
// Webapp is a webpack resolve alias pointing to the output directory, see webpack config
const url = require('Webapp/i18n/languages.json');
Copy link
Collaborator

Choose a reason for hiding this comment

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

This means the SDK now depends on Riot's Webpack config, which seems unfortunate. Maybe we can wrap this in a try and fallback to the i18nFolder version for non-Webpack/non-Riot usage?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point - this also makes the unit tests work without extra config

@jryans
Copy link
Collaborator

jryans commented Mar 1, 2019

Overall, this approach does look simpler! My main concern is about the hard dependency on Riot's Webpack, which I think we need some kind of fallback for.

dbkr added 3 commits March 1, 2019 11:44
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

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

Looks good to me, assuming CI agrees! Seems easier to work with now.

(Thanks for indulging my naming nit... 😁)

@dbkr dbkr merged commit 4f430db into release-v1.0.2 Mar 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants