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

true is not always true #5

Open
patrick-steele-idem opened this issue Sep 16, 2014 · 12 comments · May be fixed by #34
Open

true is not always true #5

patrick-steele-idem opened this issue Sep 16, 2014 · 12 comments · May be fixed by #34

Comments

@patrick-steele-idem
Copy link

I ran into problems when trying to use this module. I added the following code for debugging:

var t = require('true');
var myTrueValue = t();
console.log(myTrueValue === true);

I expected to see the program output true, but I instead saw false!

After spending hours tracing the problem down I found that another library was doing the following:

require.cache[require.resolve('true')].exports = function() {
    return false;
};

I'm not sure why this particular third party library is doing this, but I feel like something should be done to prevent it since truthiness being correct is very important for any application.

@kentcdodds
Copy link

Yep, and I think it's definitely the responsibility of this true library to ensure that it handles every single edge case and odd usage/impact from other third parties...

@patrick-steele-idem
Copy link
Author

Yes, exactly! What we need is a way to make true immutable. Since JavaScript doesn't support such a thing, I came up with the following solution:

setInterval(function() {
    if (require('true')() !== true) {
        // Fix it!
        require.cache[require.resolve('true')].exports = function() {
            return true;
        };
    }
}, 10);

This guarantees that true will always be returned even if some code tries to redefine it. I would be happy to submit a pull request.

@mde
Copy link
Owner

mde commented Sep 16, 2014

This is brilliant, although I wonder if perhaps the correct place to fix this problem is at the language-level. Perhaps you should submit something like this solution directly to TC39: http://www.ecma-international.org/memento/TC39.htm They have a long history of being incredibly receptive to outside feedback.

@patrick-steele-idem
Copy link
Author

Good idea. I will also include a recommendation to fix truthiness in JavaScript. I can't begin to tell you how many times I have been bitten by the following problem with JavaScript:

assert.equal(true, 'true'); // Error!

Clearly they are the same... It's issues like this that almost make we want to switch back to Bash for all of my programming needs.

@Ginden
Copy link

Ginden commented Mar 25, 2016

Object.defineProperty(require.cache, require.resolve('true'), {
   writable: false,
   configurable: false,
   enumerable: true,
   value: module.exports
})

@mde
Copy link
Owner

mde commented Mar 27, 2016

This could be really useful for ensuring the integrity of this module. A PR would be helpful to solicit comments, to make sure we get adequate feedback about this potential approach.

@mariomac
Copy link

Is this serious or just joke?

@mde
Copy link
Owner

mde commented Apr 25, 2016

What could possibly be funny about tiny modules? The good thing about tiny modules is that they're composable!

@amigrave
Copy link

I think this function should be available in v8

@kpatterson-klick
Copy link

I'm so grateful it's 2022 and all of the npm issues which were so problematic in 2014 have been solved forever. Truly it's a great time to be alive.

@tj-commits tj-commits linked a pull request May 30, 2024 that will close this issue
@tj-commits
Copy link
Contributor

Hey guys, I included @Ginden 's fix in my PR #34

@tj-commits
Copy link
Contributor

I also included many other crucial fixes such as misexamples in the README.md and following the single responsibility principle and getting some things off this package's hands

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 a pull request may close this issue.

8 participants