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

Check Intl config for inputPath #95

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

omairvaiyani
Copy link

Checks for inputPath option in config/ember-intl.js.

I've also pulled in the change from #94 as I couldn't run the code without it.

@Turbo87
Copy link
Collaborator

Turbo87 commented Nov 25, 2019

out of curiosity, why are you using a different path?

@omairvaiyani
Copy link
Author

omairvaiyani commented Nov 25, 2019

We use bit to share and edit files between projects, store them (including our translations) under a subdirectory. This lets our front-end team make changes to our translation files during development, and keep those changes in sync for our back-end team without having to manage multiple npm repositories/releases.

In either case, given that this option is available to intl users, its a good idea to support it in the analyzer?

@Turbo87 Turbo87 added the enhancement New feature or request label Dec 23, 2019
@Turbo87 Turbo87 self-requested a review December 23, 2019 08:56

let config;
if (fs.existsSync(configPath)) {
let requireESM = require('esm')(module);
Copy link
Collaborator

Choose a reason for hiding this comment

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

the default ember-intl blueprint is using module.exports (not export default), so I'm not sure why we would need to support ES module files here.

also, I would assume that the default export would still be a function, but it seems that the code below does not call the default and assumes that it is an object directly, right?

Copy link
Author

Choose a reason for hiding this comment

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

Have ember-intl changed their setup recently because this PR worked when I opened it

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure. I've checked in a few of my projects that use ember-intl and they all use module.exports 😅

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 this pull request may close these issues.

2 participants