Underscore: Version 1.10.2 breaking change with require.js

Created on 14 May 2020  路  14Comments  路  Source: jashkenas/underscore

Hello,

Description:
I still have an issue with the change made for 1.10.2
We try to import it with require.js and _ is undefined (from _.noConflict()).
The same codebase was working 7 weeks ago

Failling code:

require.config({
  [..]
  paths: {
    'underscore-min': '/mizar/node_modules/underscore/underscore-min',
  },
  shim: {
    'underscore-min': {
      exports: '_',
      init() {
        // eslint-disable-next-line no-undef
        return _.noConflict()
      },
    }
  },
})

Workaround:
I pinned underscore to 1.9.2 to make it work again.

1min reproduce:

git clone https://github.com/MizarWeb/Mizar
cd Mizar/
npm i
npm run build:prod

Now you can open the file Mizar/examples/test.html in your browser
sdfqsdf

fixed invalid

All 14 comments

@serut I think your setup will work with 1.10.2 if you remove the shim config. Would you mind trying that?

Even if it works without the shim config, it still sounds like a breaking change that we should fix, no?

It is a breaking change, but in this case, I don't think it should be fixed. The change is that _.noConflict is absent in AMD environments, but you don't need it anymore in AMD environments because the _ object isn't assigned to global._ anymore in AMD environments, either. Which, yes, is a breaking change as well. Both of these changes are consequences of leaving the UMD wrapper to Rollup.

I should add to this that AMD has been supported in Underscore since version 1.6 (2014). So _.noConflict and shim configs have been unnecessary for Underscore in AMD environments since that time. For this reason, I don't think many people will miss it.

I agree it is important not to needlessly break things, but in this case, adding extra code to Underscore so that people can continue to apply a workaround that has been unnecessary for six years just seems a bit too backwards.

@serut I think your setup will work with 1.10.2 if you remove the shim config. Would you mind trying that?

No that's not working to remove the shim config from the rconfig.js. It just crash with _ equals undefined on the following call :

Uncaught TypeError: Cannot read property 'find' of undefined
    at _extractURLFrom (Mizar.min.js:73062)
    at _getMizarAPIBaseURL (Mizar.min.js:73083)
    at _createConfiguration (Mizar.min.js:73116)
    at new Mizar (Mizar.min.js:72831)
    at test.js:2

If I remove this from the shim object inside rconfig.js :

    'underscore-min': {
      exports: '_',
      init() {
        // eslint-disable-next-line no-undef
        return _.noConflict()
      },
    },

It get the same error with 1.9.2. So the shim option was previously required to be able to interact with your library :expressionless:

@serut Is your code somewhere trying to use _ as a global instead of as an AMD import, or is the underscore-min module actually failing to export? These are two fundamentally different levels of breaking change.

What I do :

This works for me with all Underscore versions from 1.6.0 to 1.10.2, without shim config:

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="utf-8">
  <title>Underscore AMD reduced testcase</title>
</head>
<body>
  <script src="https://cdn.jsdelivr.net/npm/[email protected]/require.js"></script>
  <script>
    require.config({
      paths: {
        underscore: 'https://cdn.jsdelivr.net/npm/[email protected]/underscore-min'
      }
    });
    require(['module.js'], function() {});
  </script>
</body>
</html>
// module.js
define(['underscore'], function(_) {
  console.log(_.find); // prints a minified function
});

So we need to figure out what is different about your situation. I'm going to close-read your code now.

@serut When I try running your code without the shim config, everything seems fine with both 1.9.2 and 1.10.2. However, you showed an error from a failing test suite. I cannot find this test.js file that appears at the bottom of that stack trace. Also, the test script in your package.json appears to be a no-op. How do I run your tests?

Thanks for your time @jgonggrijp ! I really appreciate it.

I get what you mean, there is some confusion sorry about that ! To test it, you need to open the page examples/test.html in your browser (and not running npm test). Let me resume what I do:

  • npm i
  • I edit src/rconfig.js to remove underscore-min from shim object
  • npm run build:dev (It creates the mizar.min.js unminified)
  • I open examples/test.html in my browser

Then to make it work back:

  • I pin the version in the package.json to "underscore": "1.9.2",.
  • I rollback the file src/rconfig.js, and put back underscore-min in the shim object
  • I reinstall everything

Now I see the Milky Way
qsdfqsdf

Alright! I figured out what is happening. Your config is trying to rename the underscore module to 'underscore-min':

https://github.com/MizarWeb/Mizar/blob/837d04e9d44204519844883dfd2d9b97e59711ab/src/rconfig.js#L30

But Underscore doesn't support this, as its AMD module name has been hardcoded to 'underscore' since version 1.6.0:

https://github.com/jashkenas/underscore/blob/1f4bf626f23a99f7a676f5076dc1b1475554c8f7/underscore.js#L1331-L1342

So the require.js optimizer ends up creating an empty shim for the 'underscore-min' module, even though the actual minified 'underscore' module is also inlined:

define('underscore-min', [], function() {});

Your shim config was overriding this empty shim, which turned it into the following:

define('underscore-min', [], function() {
     return _.noConflict();
});

So in this way, it relied both on window._ and _.noConflict in order to be able to rename the 'underscore' module. This broke down in version 1.10.0.

There are three possible ways to fix this:

  1. In your code, you can use 'underscore' instead of 'underscore-min' as the module name. No shim config is needed in this case. This is the only module name that Underscore has ever supported.
  2. In Underscore, we could remove the hardcoded AMD module name. This would enable users to name the module however they wish. In this case, no shim config is required, either. However, the comment above the AMD define call gives a motivation for hardcoding the module name, so this might break things for some other people. I should mention that the require.js documentation warns against hardcoding the module name.
  3. Alternatively in Underscore, we could put back global namespace pollution and ensure that _.noConflict is present again in AMD environments. This would restore the old behavior, where using a different name is not supported, but people can work around it with a shim config. This would be my least preferred solution by far, as I think it is really, really ugly.

@jashkenas Please let me know what you think.

Thanks @jgonggrijp ! It works. I will contact the owner of that repository to use underscore instead.
I don't think the 3rd option needs to be considered, as this issue would help others doing the same thing _if they exist_.

I think you鈥檝e solved this one quite satisfactorily. 馃憦

Was this page helpful?
0 / 5 - 0 ratings

Related issues

clouddueling picture clouddueling  路  3Comments

markvr picture markvr  路  3Comments

haggholm picture haggholm  路  8Comments

jezen picture jezen  路  8Comments

githublyp picture githublyp  路  3Comments