Skip to content
This repository was archived by the owner on Oct 19, 2022. It is now read-only.

Disable loading constants module #39

Merged
merged 2 commits into from
Jan 31, 2018
Merged

Disable loading constants module #39

merged 2 commits into from
Jan 31, 2018

Conversation

damianstasik
Copy link

This PR lets you implicitly import index.js file inside constants directory, where before a node module would load with native constants.

Before:

import { TEST_CONSTANT } from 'constants/index';

After:

import { TEST_CONSTANT } from 'constants';

@papermana
Copy link

Great, that's going to make some imports look that much nicer!

I'm only concerned that it might break something, e.g. make it impossible for projects that require node constants (though I don't know why they would) to work.

Maybe we could optionally control this via an ENV var? E.g. constants: FOO_BAR === 'true'.

@damianstasik
Copy link
Author

I was about to suggest envs, after a quick search I'm pretty sure that without option to opt-out that change would break some projects. What do you think about an env named: DISABLE_NODE_CONSTANTS?

@papermana
Copy link

Sounds good, although I think that most projects might prefer opt-in instead.

@damianstasik
Copy link
Author

Opt-in as in disabling constants or enabling them? DISABLE_NODE_CONSTANTS would not break existing projects that are using native constants for some reason.

@papermana
Copy link

I meant as in disabling them by default, and allowing projects that use them to opt-in to keep them.

@damianstasik
Copy link
Author

@papermana Done, to bring back default behavior you can now use USE_NODE_CONSTANTS.

Copy link

@papermana papermana left a comment

Choose a reason for hiding this comment

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

👍

@papermana papermana merged commit 0b4196f into netguru:master Jan 31, 2018
@papermana papermana mentioned this pull request Mar 9, 2018
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.

3 participants