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

Uncaught ReferenceError: n is not defined #228

Closed
fabb opened this issue Jan 17, 2022 · 21 comments
Closed

Uncaught ReferenceError: n is not defined #228

fabb opened this issue Jan 17, 2022 · 21 comments

Comments

@fabb
Copy link

fabb commented Jan 17, 2022

I'm using image-blob-reduce in a next.js app. In dev mode it works fine, but in prod mode, I get this error:

Uncaught ReferenceError: n is not defined

Screenshot 2022-01-17 at 16 30 43

It happens in the pica source:

Screenshot 2022-01-17 at 17 05 08

I already tried turning off Terser, did not change anything.

Do you have any ideas what I could change to get this to work?

@puzrin
Copy link
Member

puzrin commented Jan 17, 2022

I guess, this is known issue with minifier https://github.com/nodeca/image-blob-reduce#known-issues

I can not debug your production bundling process, only explain principle.

@fabb
Copy link
Author

fabb commented Jan 17, 2022

I tried that already, didn’t help. I even removed Terser from the webpack plugin chain completely, same issue. I‘ll try again and debug some more and see if I haven’t made a mistake in the config or understood something wrong about the webpack config. But it has to be something that is different between dev and prod build, so the minifier would be a hot tip.

@puzrin
Copy link
Member

puzrin commented Jan 17, 2022

IMO, questions/discussions about external bundler tools are a bit out of this tracker scope. I can help only if something in dist/ is broken.

@fabb
Copy link
Author

fabb commented Jan 17, 2022

Fair enough

@puzrin
Copy link
Member

puzrin commented Jan 17, 2022

I'm closing this, but if you find anything to mention on readme - let me know.

@puzrin puzrin closed this as completed Jan 17, 2022
@fabb
Copy link
Author

fabb commented Jan 18, 2022

I'll just document my investigation process here until I find a solution.

The runtime error is that variable n is not defined in the output of my next.js webpack build process:

Screenshot 2022-01-17 at 17 05 08

    for (var o = n, u = 0; u < a.length; u++)
        i(a[u]);
    return i

The source of this compiled code is this line in the pica dist:

https://github.com/nodeca/pica/blob/master/dist/pica.js#L8

    for (var u = 'function' == typeof require && require, i = 0; i < t.length; i++)
        o(t[i]);
    return o

So somehow, webpack extracts 'function' == typeof require && require into a variable n that is nowhere defined. Not yet sure why.

The line is generated by browserify as far as I can tell, I'm not yet sure why browserify needs to check if require exists in code that runs in the browser since browsers don't implement that natively. I can imagine that webpack eliminates all occurrences of require to make the code isomorphic, but I need to investigate more on this and also find out why it only affects prod builds and not dev builds.

@fabb
Copy link
Author

fabb commented Jan 18, 2022

Curiously, image-blob-reduce does not use browserify but rather rollup to build the the lib. It's code contains similar code to the output of browserify, but with commonjsRequire instead of require:

https://github.com/nodeca/image-blob-reduce/blob/master/dist/image-blob-reduce.js#L65

for(var u="function"==typeof commonjsRequire&&commonjsRequire,i=0;i<t.length;i++)o(t[i]);

And commonjsRequire is a defined function:

	function commonjsRequire (path) {
		throw new Error('Could not dynamically require "' + path + '". Please configure the dynamicRequireTargets or/and ignoreDynamicRequires option of @rollup/plugin-commonjs appropriately for this require call to work.');
	}

So maybe there is a problem in how browserify creates the lib and how that interacts with webpack.

@fabb
Copy link
Author

fabb commented Jan 18, 2022

Interestingly, in my dev build, image-blob-reduce.esm.mjs includes the code of pica in the same file, and it replaces require with commonjsRequire:

for(var u="function"==typeof commonjsRequire&&commonjsRequire,i=0;i<t.length;i++)o(t[i])

Screenshot 2022-01-18 at 09 41 52

@fabb
Copy link
Author

fabb commented Jan 18, 2022

Ok, so I'm using next.js 12 which should prioritize ES Modules over CommonJS. image-blob-reduce.esm.mjs is an ES Module and is correctly used in my dev build, and it uses no require, only commonjsRequire (except for this string which I hope will not be a follow-up issue: https://github.com/nodeca/pica/blob/master/dist/pica.js#L1822).
But in my prod build, it looks like image-blob-reduce.esm.mjs is not used, and rather pica/dist/pica.js, so image-blob-reduce/index.js seems to be used. (Update: It is used, I was mistaken)
I need to check why the ES Module is not used for the prod build, maybe a bug in next.js, or an issue of the package.json of image-blob-reduce:

  "files": [
    "index.js",
    "lib/",
    "dist/"
  ],
  "exports": {
    "./package.json": "./package.json",
    "./lib/image_traverse.js": "./lib/image_traverse.js",
    ".": {
      "import": "./dist/image-blob-reduce.esm.mjs",
      "require": "./index.js"
    }
  },
  "module": "./dist/image-blob-reduce.esm.mjs",

@fabb
Copy link
Author

fabb commented Jan 18, 2022

I've added this question to the next.js ESM discussion thread: vercel/next.js#27876 (comment)

@fabb
Copy link
Author

fabb commented Jan 18, 2022

I tried replacing import ImageBlobReduce from 'image-blob-reduce' with import ImageBlobReduce from 'node_modules/image-blob-reduce/dist/image-blob-reduce.esm.mjs' in my client code, but that didn't change anything. I'm not yet sure if the webpack setup then force-picks the non-ES version or why this happens. (Update: the ES version is picked, that's not the issue)

@fabb
Copy link
Author

fabb commented Jan 18, 2022

Ok I was wrong about the ES version. It looks like the prod build does pick the ES version after all. I tested it by deleting the index.js and a few other files from my local node_modules folder and executing the build. The output was the same. So it must be something else.

If the ES version is used, then the code fragment for (var o = n, u = 0; u < a.length; u++) in the build output stems from this line in image-blob-reduce.esm.mjs: https://github.com/nodeca/image-blob-reduce/blob/master/dist/image-blob-reduce.esm.mjs#L59

...for(var u="function"==typeof commonjsRequire&&commonjsRequire,i=0;i<t.length;i++)...

So commonjsRequire is our n. commonjsRequire is defined at the top of the file: https://github.com/nodeca/image-blob-reduce/blob/master/dist/image-blob-reduce.esm.mjs#L3

function commonjsRequire (path) {
	throw new Error('Could not dynamically require "' + path + '". Please configure the dynamicRequireTargets or/and ignoreDynamicRequires option of @rollup/plugin-commonjs appropriately for this require call to work.');
}

This function is contained in my build output as function n(e):

...434:function(e,t,r){"use strict";function n(e){throw new Error('Could not dynamically require "'+e+'". Please configure the dynamicRequireTargets or/and ignoreDynamicRequires option of @rollup/plugin-commonjs appropriately for this require call to work.')}r.d(t,{Z:function(){return g}});var a={};function i(e,t){var r={};return t.forEach((function(t){Object.prototype.hasOwnProperty.call(e,t)&&(r[t]=e[t])})),r}a.assign=function(e){for(var t,r=1;r<arguments.length;r++)for(var n in t=Object(arguments[r]))Object.prototype.hasOwnProperty.call(t,n)&&(e[n]=t[n]);return e},a.pick=i,a.pick_pica_resize_options=function(e){return i(e,["alpha","unsharpAmount","unsharpRadius","unsharpThreshold","cancelToken"])};var o={exports:{}};!function(e,t){e.exports=function(){function e(t,r,a){function i(u,s){if(!r[u]){if(!t[u]){if(!s&&n)return n(u);if(o)return o(u,!0);var c=new Error("Cannot find module '"+u+"'");throw c.code="MODULE_NOT_FOUND",c}var l=r[u]={exports:{}};t[u][0].call(l.exports,(function(e){return i(t[u][1][e]||e)}),l,l.exports,e,t,r,a)}return r[u].exports}for(var o=n,u=0;u<a.length;u++)i(a[u]);...

Prettified:

    434: function(e, t, r) {
        "use strict";
        function n(e) {
            throw new Error('Could not dynamically require "' + e + '". Please configure the dynamicRequireTargets or/and ignoreDynamicRequires option of @rollup/plugin-commonjs appropriately for this require call to work.')
        }
        r.d(t, {
            Z: function() {
                return g
            }
        });
        var a = {};
        function i(e, t) {
            var r = {};
            return t.forEach((function(t) {
                Object.prototype.hasOwnProperty.call(e, t) && (r[t] = e[t])
            }
            )),
            r
        }
        a.assign = function(e) {
            for (var t, r = 1; r < arguments.length; r++)
                for (var n in t = Object(arguments[r]))
                    Object.prototype.hasOwnProperty.call(t, n) && (e[n] = t[n]);
            return e
        }
        ,
        a.pick = i,
        a.pick_pica_resize_options = function(e) {
            return i(e, ["alpha", "unsharpAmount", "unsharpRadius", "unsharpThreshold", "cancelToken"])
        }
        ;
        var o = {
            exports: {}
        };
        !function(e, t) {
            e.exports = function() {
                function e(t, r, a) {
                    function i(u, s) {
                        if (!r[u]) {
                            if (!t[u]) {
                                if (!s && n)
                                    return n(u);
                                if (o)
                                    return o(u, !0);
                                var c = new Error("Cannot find module '" + u + "'");
                                throw c.code = "MODULE_NOT_FOUND",
                                c
                            }
                            var l = r[u] = {
                                exports: {}
                            };
                            t[u][0].call(l.exports, (function(e) {
                                return i(t[u][1][e] || e)
                            }
                            ), l, l.exports, e, t, r, a)
                        }
                        return r[u].exports
                    }
                    for (var o = n, u = 0; u < a.length; u++)
                        i(a[u]);
                    return i
                }
                return e
            }()({
                1: [function(e, t, r) {
                    var n = e("multimath")
                      , a = e("./mm_unsharp_mask")
                      , i = e("./mm_resize");

When the page initially loads, this code executes fine, also the for (var o = n, u = 0; u < a.length; u++). Only when I initialize the instance of image-blob-reduce in my code, the Uncaught ReferenceError: n is not defined happens. I seem not to be able to make it stop at a breakpoint.

What looks weird is that image-blob-reduce.esm.mjs contains commonjsRequire. I'm not sure if ES Modules are supposed to contain such nested CommonJS modules. Maybe that trips something up.

@fabb
Copy link
Author

fabb commented Jan 21, 2022

@puzrin I could get it to work, but not as-is. I needed to modify the pica and image-blob-reduce libs in this way:

  • Added an esm build with rollup to pica. Note that i left out babel for simplicity for now. And note that I only specified module in the package.json and not exports on purpose, because that would cause issues and make image-blob-reduce pick up the non-esm version because it uses requires.
  • Removed webworkify from pica as a quick fix because it uses arguments which later causes a build error in the client project. It also uses some weird require magic which I don't know if it would work when not built with browserify.
  • Activated the requireReturnsDefault option for the commonjs plugin in the rollup config of image-blob-reduce.
  • Built image-blob-reduce with the pica esm module from my branch.

You can see the code changes here:

And this is the working image-blob-reduce esm module: https://github.com/fabb/image-blob-reduce/blob/pica-esm/dist/image-blob-reduce.esm.mjs

I did not test the non-esm build outputs, so I don't know if they would still work as expected.

Would you be interested in integrating these changes into your packages? The webworkify part would need to be figured out still, and the non-esm libs of image-blob-reduce would need to be tested.

@fabb
Copy link
Author

fabb commented Jan 21, 2022

Ah I see you already tried to find an alternative for webworkify 😄
rollup/rollup#1029 (comment)

@puzrin
Copy link
Member

puzrin commented Jan 21, 2022

Yes, we stay with browserify because other bundlers do not support webworkers in single file. I can not accept use of another bundler to build pica internals until this solved.

@fabb
Copy link
Author

fabb commented Jan 22, 2022

I understand. Too bad that combining browserify + rollup + webpack seems to be problematic 😅
Out of curiosity: is code-deduplication a requirement too, or could the code be duplicated in a string literal in the same file to be used by the web worker?

@puzrin
Copy link
Member

puzrin commented Jan 22, 2022

Too bad that combining browserify + rollup + webpack seems to be problematic

That works. I don't know why you have problems.

Out of curiosity: is code-deduplication a requirement too, or could the code be duplicated in a string literal in the same file to be used by the web worker?

At current moment i don't know good alternatives to existing approach. Everything was already considered.

@fabb
Copy link
Author

fabb commented Jan 22, 2022

Too bad that combining browserify + rollup + webpack seems to be problematic

That works. I don't know why you have problems.

It might work with vanilla webpack, but not with vanilla next.js. I created a reproduction codesandbox with the default next.js 12 config: vercel/next.js#33452

It might be related to the webpack config that comes with next.js, I guess it‘s some issue with tree shaking since that is different between prod and dev builds in webpack.

@flavianh
Copy link

Coming here to say that I've got the same issue as @fabb . The best solution right now seems to be monkeypatching nextjs, which isn't great vercel/next.js#30924

@non25
Copy link

non25 commented Mar 21, 2023

I ended up using patch-package to force using source version.
Works fine so far.

diff --git a/node_modules/image-blob-reduce/package.json b/node_modules/image-blob-reduce/package.json
index f5fcea1..6c66f3b 100644
--- a/node_modules/image-blob-reduce/package.json
+++ b/node_modules/image-blob-reduce/package.json
@@ -21,7 +21,7 @@
     "./package.json": "./package.json",
     "./lib/image_traverse.js": "./lib/image_traverse.js",
     ".": {
-      "import": "./dist/image-blob-reduce.esm.mjs",
+      "import": "./index.js",
       "require": "./index.js"
     }
   },

@chaudev
Copy link

chaudev commented Sep 24, 2024

I use nextjs 14, this worked for me

import ImageBlobReduce from "image-blob-reduce";
import Pica from "pica";

const pica = Pica({ features: ["js", "wasm", "cib"] });
const reduce = new ImageBlobReduce({ pica });

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

No branches or pull requests

5 participants