Code Monkey home page Code Monkey logo

Comments (7)

jrburke avatar jrburke commented on August 19, 2024

I'm not sure this is worth the extra characters in the file -- in the interests of keeping almond small, I'm tending to just leave the code as-is, since the caller will get an undefined module anyway. I know this is not strictly as helpful as it could be, but I want to balance code weight. Going to close for now, but we can still continue the discussion and reopen if necessary.

from almond.

pmuellr avatar pmuellr commented on August 19, 2024

Yes, it is most definitely worth the extra characters in the file.

The problem is that this is always a failure case, unless for some reason you aren't dependent on the side effects of require()ing the module or the returned module exports. In which case, it's still a failure, since that code can be safely removed from your code path.

So, it's always a failure. Always. But, with the current version of code, developers aren't informed of the failure till they dereference the result of require(). Which might be quite distant, in both lines of code, and elapsed time, from when the require() occurred.

Now, even though I'm ashamed to admit this, I have been known, on rare occasions, to accidentally inject bugs into my code. This even happened last week! Guess what I did? I spelled the name of a module incorrectly, in a require() function. And it took me a minute or two to track down what was happening, since the null dereference occurred somewhere not close to the require() invocation.

It should have taken me no minutes to do this. I should have seen the error immediately. Woulda saved me a couple minutes!

Please read: http://martinfowler.com/ieeeSoftware/failFast.pdf

from almond.

christophercurrie avatar christophercurrie commented on August 19, 2024

Perhaps a compromise approach would be to have a debug version of almond.js that does this check? Usually I'm less concerned about code size when I'm still in development.

from almond.

jrburke avatar jrburke commented on August 19, 2024

@christophercurrie I think a debug version would complicate the use of almond, it is nice to just have the one file. Here are my concerns:

  1. there could be a case where for regular scripts that do not call define() are built in to the layer, and therefore the require('scriptname') would not have a define()'d value, but it is OK, because the point was to just indicate the need to load the file, since a global will be used to access the functionality. This comes out of require.js supporting loading of plain script tags.

Although, it is probably enough to say "make sure an empty define('scriptname', function(){}) is inserted for those kinds of scripts", and the r.js optimizer does this by default. There is an option to turn that off though. But anyway, this does not seem to be a big blocker, and almond should err on the side of encouraging modular development.

  1. A logical extension of what @christophercurrie is suggesting, I'm sure there are other checks that could be done, but at the cost of increasing the size of the code. So the trick is choosing the ones that would trip up the developer most and cause the worst wastes of time. This one might fit in that category.

I'll reopen this ticket for now to indicate it is still under consideration.

from almond.

jrburke avatar jrburke commented on August 19, 2024

This was partly fixed in 0.1 for dependency arrays, but was rounded out in 0.1.1.

from almond.

mikemorris avatar mikemorris commented on August 19, 2024

@jrburke This behavior is inconsistent between requirejs and almond (requirejs will not throw an exception). There are some valid cases where a dependency could be undefined, such as a UMD module (https://github.com/umdjs/umd) with slightly different dependencies between AMD and CommonJS environments. The workaround suggested at #12 (comment) can be modified slightly to be an undefined module adapter, but this doesn't feel like a proper solution.

define(function () {
    return null;
});

from almond.

jrburke avatar jrburke commented on August 19, 2024

@mikemorris I'm not sure I follow. This ticket was throwing on a module not included in the built file. Given that almond only works with all the modules built in, throwing makes sense. For requirejs, it would trigger a dynamic load. However, requirejs expects to load a value, and will throw a type of error if that dynamic load fails. It will just be an error that happens in a later turn, and could be a "timeout" error depending on how requirejs is configured.

Does this sound like the build situation: you are building something using almond, and find you need to insert a dummy module for a dependency you do not need in the almond-built version. If so, you can use the r.js stubModules to put in an empty define call for that module. But there will need to be some define() registered for that module ID.

from almond.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.