mishoo / UglifyJS

JavaScript parser / mangler / compressor / beautifier toolkit
http://lisperator.net/uglifyjs/
Other
13.16k stars 1.25k forks source link

vercel/next lambda fails to run minified bundle. #4327

Closed talmobi closed 3 years ago

talmobi commented 3 years ago

Bug report or feature request?

Bug

Uglify version (uglifyjs -V)

3.10.2

JavaScript input

NONE

The uglifyjs CLI command executed or minify() options used.

uglifyjs dist/yt-search.js -cmo dist/yt-search.min.js

JavaScript output or error produced.

Vercel/next lambda is unable to resolve dependencies from a minified bundle: https://i.imgur.com/2XST3ox.png

The non-minified bundle works fine without errors inside the vercel/next lambda: https://vercel-next-yt-search-test-127g4hwhu.vercel.app/api/hello

The minified bundle loads and works fine within node itself, however.

This issue was discovered here: https://github.com/talmobi/yt-search/issues/14

It is unclear if it's a vercel/next issue, or an uglify-js issue.

I've been in contact with vercel support but nothing significant has come out of it as of yet.

alexlamsl commented 3 years ago

Where can I access this dist/yt-search.js you have specified?

Also, please try latest version of uglify-js too see if the issue persists first.

talmobi commented 3 years ago

Just tried uglify-js@3.12.1 and the same error happens.

pastebin for dist/yt-search.js: https://pastebin.com/raw/Nmj1tLpy

alternatively you can build it yourself by cloning repo:

git clone --branch talmobi/fix/vercel-next-test https://github.com/talmobi/yt-search cd yt-search && npm install && npm run build && ls dist/

and the vercel/next app that I'm using to test is here: https://github.com/talmobi/vercel-next-yt-search-test

The error occurs in the pages/api/hello.js which is a lambda function. In pages/index.js it works fine as it's a normal node environment.

alexlamsl commented 3 years ago

So I tried the pastebin content on command line:

$ node yt-search.js
internal/modules/cjs/loader.js:883
  throw err;
  ^

Error: Cannot find module 'cheerio'
Require stack:
- yt-search.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)
    at Function.Module._load (internal/modules/cjs/loader.js:725:27)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at o ([stdin]:1:493)
    at [stdin]:1:684
    at Object.1../util.js ([stdin]:6:16)
    at o ([stdin]:1:633)
    at r ([stdin]:1:799)
    at [stdin]:1:828 {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ 'yt-search.js' ]
}

which is the same as the screenshot you've linked above.

Unless I am mistaken − yt-search.js you've linked above should not give me any error?

alexlamsl commented 3 years ago

Same error being thrown with dist/yt-search.js from:

$ git clone --branch talmobi/fix/vercel-next-test https://github.com/talmobi/yt-search
alexlamsl commented 3 years ago

Looking inside those original input, i.e. before uglify-js processes them, they have these lines:

{
  "./util.js": 2,
  "async.parallellimit": undefined,
  "cheerio": undefined,
  "dasu": "dasu",
  "fs": undefined,
  "human-time": undefined,
  "jsonpath": undefined,
  "path": undefined,
  "querystring": undefined,
  "url": undefined
}

I'm wondering if those should be numbers instead of undefined? 🤔

talmobi commented 3 years ago

It's the bundle of the source code but it still depends on a few dependencies such as cheerio.

It's not something you should be able to run from the command line directly, but be able to require ala CommonJS and use in some javascript.

const yts = require( `./yt-search.js` )
...

It should work, I just tried the pastebin version and it works.


And to answer the undefined part that's to indicate the modules aren't packed into the bundle itself (that they are external dependencies that should be found in node and node_modules). It's something browserify does.


The non-minified bundle works fine in vercel/next lambda, but the minified version does not.


If you cloned the branch/repo you should be able to: npm install && npm test which runs the test cases for the bundles without problems.

alexlamsl commented 3 years ago

The procedure you gave me isn't a step-by-step that leads to a PASS/FAIL scenario, so please bear with me while I try to understand all those words in between.

I have tried the following:

$ git clone https://github.com/talmobi/vercel-next-yt-search-test
$ cd vercel-next-yt-search-test
$ npm install
$ npm run build
$ npm run start

> nextjs@0.1.0 start vercel-next-yt-search-test
> next start

ready - started server on http://localhost:3000

After that I use a web browser to navigate to http://localhost:3000/api/hello, which produce the following in the console:

api called ended
title: AGA 江海迦﹣《Superman》MV
api called ended

and returns the following JSON reply:

{"name":"AGA 江海迦﹣《Superman》MV"}

So assuming the above is the successful case, how do I make it fail?

alexlamsl commented 3 years ago

... and looking at node_modules/yt-search/package.json, it seems to be using the minified version of yt-search already:

{
  "_from": "github:talmobi/yt-search#talmobi/fix/vercel-next-test",
  ...
  "main": "dist/yt-search.min.js",
  ...
}
talmobi commented 3 years ago

Correct, it all works locally.

The issue happens when you upload this vercel/next application to vercel.com.

The code inside pages/api/* (in our case pages/api/hello.js) are uploaded by vercel internally as lambda functions.

The issue is that the hosted version fails to run the code inside the lambda function.

If you use the non-minified version of the bundle (dist/yt-search.js) it works, as you can see in this hosted url: https://vercel-next-yt-search-test-127g4hwhu.vercel.app/api/hello

But if you use the minifed version (dist/yt-search.min.js) it fails with the above error messages (failing to resolve modules). You can see this message in this url (NOTE: this url is hosting a different version of the app that is using the minified bundle): https://vercel-next-yt-search-test-hw3khp7dl.vercel.app/api/hello

The in-depth error log is screenshotted from the vercel.com app page is on the first post of this issue (ie this one: https://i.imgur.com/2XST3ox.png )

It's a bit troublesome to test because it only fails when it's hosted on vercel, but hosting there is free. Again I'm not sure if this is an issue with how vercel/next resolves the dependencies for their lambda or an issue with uglify-js -- but the non-minified version works fine.

alexlamsl commented 3 years ago

Ah I see − thanks for the clarification.

I'm afraid I'm out of ideas about how to reproduce this then, since I cannot access the code hosted remotely (they aren't even client-side scripts 😓) to tweak and see what works/breaks.

Before I ponder further − do you upload the whole of git clone https://github.com/talmobi/vercel-next-yt-search-test onto that hosting service, and if not which bits are included?

talmobi commented 3 years ago

The whole https://github.com/talmobi/vercel-next-yt-search-test is uploaded there -- in fact you just reference the url and it download/handles it for you.

And because of the "yt-search": "github:talmobi/yt-search#talmobi/fix/vercel-next-test" in the above repo's code it downloads whatever the latest code is in that specific branch as the latest yt-search version that I'm experimenting with.

You should fork your own experimental branch and put your fork/branch url there instead to experiment with freely. That's the simplest way I can se right now how to do it.

The latest version in that exprimental branch of the yt-search repo does indeed reference the minified version (that doesn't work). I've been testing the minified version (like upgrading the uglify-js module) and just re-deploying the vercel/next app that would download the latest experimental yt-search version from that branch.

In the vercel/next app's that I've linked above that has the working version is using the non-minified version, they are just unique urls of previously deployed versions that are still alive.

That being said I'll see if I can make a smaller reproducible.

alexlamsl commented 3 years ago

Thanks for the detailed explanation.

I'm combing over the orignal and minified yt-search.js at the moment to look for potential clues − barring a major eureka I'll give this vercel thing a go.

Meanwhile if you don't mind, please try disabling some optimisation flags to see if it makes a difference, e.g. uglifyjs dist/yt-search.js -c inline=false -o dist/yt-search.min.js

The usual suspects I'd go for:

talmobi commented 3 years ago

Will try to create a minimal sample repo and steps of the issue.

~I tried turning those flags off/false individually but they didn't produce any difference in the minified output result. Thus they shouldn't produce any difference once being uploaded.~

talmobi commented 3 years ago

the error also persisted with or without --mangle

alexlamsl commented 3 years ago

I tried turning those flags off/false individually but they didn't produce any difference in the minified output result.

That's odd, since uglifyjs dist/yt-search.js -co dist/yt-search.min.js gives me 22,341 bytes, whereas uglifyjs dist/yt-search.js -c inline=false -o dist/yt-search.min.js produces 22,430 bytes instead.

talmobi commented 3 years ago

Yes good catch I was editing the wrong package.json... one sec...

talmobi commented 3 years ago

-c inline=false still errors: https://vercel-next-yt-search-test-63050iwyr.vercel.app/api/hello

-c collapse_vars=false still errors: https://vercel-next-yt-search-test-8c6g0ai1b.vercel.app/api/hello

-c merge_vars=false still errors: https://vercel-next-yt-search-test-9nhczlhqd.vercel.app/api/hello

-c reduce_vars=false still errors: https://vercel-next-yt-search-test-3m8aedoj9.vercel.app/api/hello

-c collapse_vars=false -c inline=false -c merge_vars=false -c reduce_vars=false still error: https://vercel-next-yt-search-test-pchgqle60.vercel.app/api/hello

and just to test the non-minified version again I reuploaded and redeployed it and it's working: https://vercel-next-yt-search-test-ec3kz1q3n.vercel.app/api/hello

EDIT: made the links clickable

alexlamsl commented 3 years ago

Thanks for all the tests!

Does this vercel scenario ever work before, i.e. was there a version of uglify-js that used to work, or is this a brand new combination?

alexlamsl commented 3 years ago

Actually, one more test − minified but no compression:

uglifyjs dist/yt-search.js -o dist/yt-search.min.js

That should be the original file with only extra whitespace and brackets removed.

talmobi commented 3 years ago

Thanks for all the tests!

Does this vercel scenario ever work before, i.e. was there a version of uglify-js that used to work, or is this a brand new combination?

I'm not sure, I only ~certainly~ recently was made aware/discovered this issue.

Actually, one more test − minified but no compression:

uglifyjs dist/yt-search.js -o dist/yt-search.min.js

That should be the original file with only extra whitespace and brackets removed.

still the same error: https://vercel-next-yt-search-test-2clpm3fg7.vercel.app/api/hello

EDIT: typo

alexlamsl commented 3 years ago

I see − in which case my current suspicion is that vercel may have some hard-coded pattern replacement of browserify's require code string for some internal optimisation purposes.

The original file has extra whitespace around = on the first line here:

g.ytSearch = f()
          ^ ^

and extra parenthesis around here:

return (function(){function r(e,n,t){
       ^

Although there is nothing wrong with the functionality of the minified code, if vercel is looking for an exact string match, then this would explain the failure we are observing here, i.e. code only fails when hosted on their platform.

yareyaredesuyo commented 3 years ago

So, is it maybe vercel's loading program's bug?

Vercel looks like using this lib https://github.com/vercel/nft internally when loading libs at runtime.

Should add this problem to nft's isssue?

talmobi commented 3 years ago

@yareyaredesuyo

I'll see if I can replicate the issue on a minimal codebase with a simpler step-by-step first, I think.

yareyaredesuyo commented 3 years ago

Following is simple way for using nft.

npm init -y
yarn add @vercel/nft@0.9.4
yarn add yt-search@2.5.1

run nft print

npx nft print ./node_modules/yt-search/dist/yt-search.js

print result

FILELIST:
node_modules/async.parallellimit/index.js
node_modules/async.parallellimit/package.json
node_modules/async.util.eachoflimit/index.js
node_modules/async.util.eachoflimit/package.json
node_modules/async.util.isarray/index.js
node_modules/async.util.isarray/package.json
node_modules/async.util.isarraylike/index.js
node_modules/async.util.isarraylike/package.json
node_modules/async.util.keyiterator/index.js
node_modules/async.util.keyiterator/package.json
node_modules/async.util.keys/index.js
node_modules/async.util.keys/package.json
node_modules/async.util.noop/index.js
node_modules/async.util.noop/package.json
node_modules/async.util.once/index.js
node_modules/async.util.once/package.json
node_modules/async.util.onlyonce/index.js
node_modules/async.util.onlyonce/package.json
node_modules/async.util.parallel/index.js
node_modules/async.util.parallel/package.json
node_modules/async.util.restparam/index.js
node_modules/async.util.restparam/package.json
node_modules/boolbase/index.js
node_modules/boolbase/package.json
node_modules/cheerio/index.js
node_modules/cheerio/lib/api/attributes.js
node_modules/cheerio/lib/api/css.js
node_modules/cheerio/lib/api/forms.js
node_modules/cheerio/lib/api/manipulation.js
node_modules/cheerio/lib/api/traversing.js
node_modules/cheerio/lib/cheerio.js
node_modules/cheerio/lib/parse.js
node_modules/cheerio/lib/static.js
node_modules/cheerio/lib/utils.js
node_modules/cheerio/node_modules/dom-serializer/index.js
node_modules/cheerio/node_modules/dom-serializer/package.json
node_modules/cheerio/package.json
node_modules/css-select/index.js
node_modules/css-select/lib/attributes.js
node_modules/css-select/lib/compile.js
node_modules/css-select/lib/general.js
node_modules/css-select/lib/procedure.json
node_modules/css-select/lib/pseudos.js
node_modules/css-select/lib/sort.js
node_modules/css-select/package.json
node_modules/css-what/index.js
node_modules/css-what/package.json
node_modules/dasu/dist/dasu.min.js
node_modules/dasu/package.json
node_modules/dom-serializer/foreignNames.json
node_modules/dom-serializer/index.js
node_modules/dom-serializer/node_modules/domelementtype/lib/index.js
node_modules/dom-serializer/node_modules/domelementtype/package.json
node_modules/dom-serializer/node_modules/entities/lib/decode.js
node_modules/dom-serializer/node_modules/entities/lib/decode_codepoint.js
node_modules/dom-serializer/node_modules/entities/lib/encode.js
node_modules/dom-serializer/node_modules/entities/lib/index.js
node_modules/dom-serializer/node_modules/entities/lib/maps/decode.json
node_modules/dom-serializer/node_modules/entities/lib/maps/entities.json
node_modules/dom-serializer/node_modules/entities/lib/maps/legacy.json
node_modules/dom-serializer/node_modules/entities/lib/maps/xml.json
node_modules/dom-serializer/node_modules/entities/package.json
node_modules/dom-serializer/package.json
node_modules/domelementtype/index.js
node_modules/domelementtype/package.json
node_modules/domhandler/index.js
node_modules/domhandler/lib/element.js
node_modules/domhandler/lib/node.js
node_modules/domhandler/package.json
node_modules/domutils/index.js
node_modules/domutils/lib/helpers.js
node_modules/domutils/lib/legacy.js
node_modules/domutils/lib/manipulation.js
node_modules/domutils/lib/querying.js
node_modules/domutils/lib/stringify.js
node_modules/domutils/lib/traversal.js
node_modules/domutils/package.json
node_modules/entities/index.js
node_modules/entities/lib/decode.js
node_modules/entities/lib/decode_codepoint.js
node_modules/entities/lib/encode.js
node_modules/entities/maps/decode.json
node_modules/entities/maps/entities.json
node_modules/entities/maps/legacy.json
node_modules/entities/maps/xml.json
node_modules/entities/package.json
node_modules/escodegen/escodegen.js
node_modules/escodegen/package.json
node_modules/esprima/esprima.js
node_modules/esprima/package.json
node_modules/estraverse/estraverse.js
node_modules/estraverse/package.json
node_modules/esutils/lib/ast.js
node_modules/esutils/lib/code.js
node_modules/esutils/lib/keyword.js
node_modules/esutils/lib/utils.js
node_modules/esutils/package.json
node_modules/htmlparser2/lib/CollectingHandler.js
node_modules/htmlparser2/lib/FeedHandler.js
node_modules/htmlparser2/lib/Parser.js
node_modules/htmlparser2/lib/ProxyHandler.js
node_modules/htmlparser2/lib/Stream.js
node_modules/htmlparser2/lib/Tokenizer.js
node_modules/htmlparser2/lib/WritableStream.js
node_modules/htmlparser2/lib/index.js
node_modules/htmlparser2/node_modules/domutils/index.js
node_modules/htmlparser2/node_modules/domutils/lib/helpers.js
node_modules/htmlparser2/node_modules/domutils/lib/legacy.js
node_modules/htmlparser2/node_modules/domutils/lib/manipulation.js
node_modules/htmlparser2/node_modules/domutils/lib/querying.js
node_modules/htmlparser2/node_modules/domutils/lib/stringify.js
node_modules/htmlparser2/node_modules/domutils/lib/traversal.js
node_modules/htmlparser2/node_modules/domutils/package.json
node_modules/htmlparser2/package.json
node_modules/human-time/human.js
node_modules/human-time/package.json
node_modules/inherits/inherits.js
node_modules/inherits/inherits_browser.js
node_modules/inherits/package.json
node_modules/jsonpath/generated/parser.js
node_modules/jsonpath/include/action.js
node_modules/jsonpath/include/module.js
node_modules/jsonpath/index.js
node_modules/jsonpath/lib/aesprim.js
node_modules/jsonpath/lib/dict.js
node_modules/jsonpath/lib/grammar.js
node_modules/jsonpath/lib/handlers.js
node_modules/jsonpath/lib/index.js
node_modules/jsonpath/lib/parser.js
node_modules/jsonpath/lib/slice.js
node_modules/jsonpath/package.json
node_modules/lodash.assignin/index.js
node_modules/lodash.assignin/package.json
node_modules/lodash.bind/index.js
node_modules/lodash.bind/package.json
node_modules/lodash.defaults/index.js
node_modules/lodash.defaults/package.json
node_modules/lodash.filter/index.js
node_modules/lodash.filter/package.json
node_modules/lodash.flatten/index.js
node_modules/lodash.flatten/package.json
node_modules/lodash.foreach/index.js
node_modules/lodash.foreach/package.json
node_modules/lodash.map/index.js
node_modules/lodash.map/package.json
node_modules/lodash.merge/index.js
node_modules/lodash.merge/package.json
node_modules/lodash.pick/index.js
node_modules/lodash.pick/package.json
node_modules/lodash.reduce/index.js
node_modules/lodash.reduce/package.json
node_modules/lodash.reject/index.js
node_modules/lodash.reject/package.json
node_modules/lodash.some/index.js
node_modules/lodash.some/package.json
node_modules/nth-check/compile.js
node_modules/nth-check/index.js
node_modules/nth-check/package.json
node_modules/nth-check/parse.js
node_modules/readable-stream/errors.js
node_modules/readable-stream/lib/_stream_duplex.js
node_modules/readable-stream/lib/_stream_passthrough.js
node_modules/readable-stream/lib/_stream_readable.js
node_modules/readable-stream/lib/_stream_transform.js
node_modules/readable-stream/lib/_stream_writable.js
node_modules/readable-stream/lib/internal/streams/async_iterator.js
node_modules/readable-stream/lib/internal/streams/buffer_list.js
node_modules/readable-stream/lib/internal/streams/destroy.js
node_modules/readable-stream/lib/internal/streams/end-of-stream.js
node_modules/readable-stream/lib/internal/streams/from.js
node_modules/readable-stream/lib/internal/streams/pipeline.js
node_modules/readable-stream/lib/internal/streams/state.js
node_modules/readable-stream/lib/internal/streams/stream.js
node_modules/readable-stream/package.json
node_modules/readable-stream/readable.js
node_modules/safe-buffer/index.js
node_modules/safe-buffer/package.json
node_modules/source-map/lib/array-set.js
node_modules/source-map/lib/base64-vlq.js
node_modules/source-map/lib/base64.js
node_modules/source-map/lib/binary-search.js
node_modules/source-map/lib/mapping-list.js
node_modules/source-map/lib/quick-sort.js
node_modules/source-map/lib/source-map-consumer.js
node_modules/source-map/lib/source-map-generator.js
node_modules/source-map/lib/source-node.js
node_modules/source-map/lib/util.js
node_modules/source-map/package.json
node_modules/source-map/source-map.js
node_modules/static-eval/index.js
node_modules/static-eval/package.json
node_modules/string_decoder/lib/string_decoder.js
node_modules/string_decoder/package.json
node_modules/underscore/package.json
node_modules/underscore/underscore.js
node_modules/util-deprecate/node.js
node_modules/util-deprecate/package.json
node_modules/yt-search/dist/yt-search.js
node_modules/yt-search/package.json

Change yt-search version

yarn add yt-search@2.5.0

run nft print

npx nft print ./node_modules/yt-search/dist/yt-search.min.js

print result

FILELIST:
node_modules/yt-search/dist/yt-search.min.js
node_modules/yt-search/package.json

hmm, look like nothing loaded..

alexlamsl commented 3 years ago

Closing since it's not an issue with uglify-js − but do feel free to continue with the discussion here.

yareyaredesuyo commented 3 years ago

For the sake of future duplication, share:

The problem is that require is mangled, so nft doesn't know about the imported dependencies.

For example, you can see that yt-search.min.js no longer has require('cheerio') like it does in the original yt-search.js.

If you can preserve the require name during minification, it should work.

Note that this will not be a problem for ESM because import is not a variable.

https://github.com/vercel/nft/issues/161#issuecomment-738841565

yareyaredesuyo commented 3 years ago

@alexlamsl

Is there no option to achieve this? just want to testing...

If you can preserve the require name during minification, it should work.

alexlamsl commented 3 years ago

Clearly it's more than just preserving the name require, since in https://github.com/mishoo/UglifyJS/issues/4327#issuecomment-737324528 it was tested with no variable renaming (or even compression) whatsoever, just whitespace (& parentheses) removal and vercel still doesn't work.