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

Unexpected token: keyword (function) UglifyJs #172

Closed
lyquocnam opened this issue May 16, 2017 · 31 comments
Closed

Unexpected token: keyword (function) UglifyJs #172

lyquocnam opened this issue May 16, 2017 · 31 comments

Comments

@lyquocnam
Copy link

hi author, i trying to use rxdb with vuejs + cordova ( quasar framework )
but when build this bug happend.

ERROR in js/0.bcc8ac658f5ce5687243.js from UglifyJs
Unexpected token: keyword (function) [js/0.bcc8ac658f5ce5687243.js:590,6]

more detail i posted here: quasarframework/quasar#573
this error make app can not run on mobile cordova, please help !

@pubkey
Copy link
Owner

pubkey commented May 16, 2017

Hi @lyquocnam
To debug this, we need to know what code is in js/0.bcc8ac658f5ce5687243.js:590,6
Can you access this file? One option would be to disable UglifyJS to produce it.

@varbrad
Copy link
Contributor

varbrad commented May 16, 2017

Hi @lyquocnam.
This error is produced by UglifyJS whenever it tries to 'uglify' a function that has been designated as an asynchronous function.

Depending on how the webpack setup is defined for the quasar-cli, you will need to run this bundle through a transpiler such as babel first. It looks like that function is being required in wholesale from rxjs somewhere.

NOTE: Offending function in your prod bundle;

async function assertThrowsAsync(test, error = Error, contains = '') { ... }

@lyquocnam
Copy link
Author

my project here https://github.com/lyquocnam/quasar-test

@rstoenescu
Copy link

Hi all,

Didn't quite look, but is there a transpiled version of Rxdb in the npm package? This would solve the problem for all projects using Webpack by default, without the need to add Webpack exceptions, which is what most devs expect.

@varbrad
Copy link
Contributor

varbrad commented May 16, 2017

Just a note that the naughty async function that is getting put into the pre-uglify file is being pulled in from rxdb/src/util.js, line 81. (That's what webpack is telling me anyway, webpack surely should never even be looking at this file?)

Looks like somewhere (rxdb?, webpack?) is pulling in the util async functions from the 'src' directory as opposed to the 'dist' transpiled asyncToGenerators?

@varbrad
Copy link
Contributor

varbrad commented May 16, 2017

Ok I think I have figured it out. Webpack is using the module source from rxdb when being built. This results in webpack just using the source files rather than the dist files.

This means the async/await stuff just gets put into the resulting bundle, and then uglify-js gets confused when it sees them when attempting to uglify the source.

Also, if babel (or similar) is running within the quasar build step, then it will be ignoring node_modules (presumably, I haven't checked) and voila, there's the issue.

@rstoenescu
Copy link

rstoenescu commented May 16, 2017

@varbrad Hi, Please read this post from Rich Harris explaining what "jsnext:main" is and how it should be used by package owners, and then the next comment. Seems like rxdb has chosen option 3, but this complicates things a lot for devs using Webpack which have to do extra configuration. You might want to chose another option so that this will work "out of the box" with Webpack too. Most users just drop a package and search for replacement when having such problems -- not to mention not everyone is a Webpack wizard :) Just my opinion.

EDIT: forgot to add link :) reduxjs/redux#1042 (comment)

@varbrad
Copy link
Contributor

varbrad commented May 16, 2017

Hi @rstoenescu, I am not the author of rxdb, that would be @pubkey. 👍
Also, unless I am being really dumb (it is very possible), I am not seeing the link you mention?
😃

@rstoenescu
Copy link

@varbrad Ah, you are right. Forgot to add the link itself :) Edited the post.

@pubkey
Copy link
Owner

pubkey commented May 16, 2017

Thank you @varbrad and @rstoenescu for the investigation.

It looks like RxDB is doing something wrong here.
Currently jsnext:main and module point to the es7-stage-0 code which seems to be the wrong way to go.
As seen on redux, we should transpile the es7 to es6 and let the jsnext:main point to the es6-version.

Would this fix the issue or do you see any further problems with this?

@varbrad
Copy link
Contributor

varbrad commented May 16, 2017

I think that is the way to go, by exporting a CommonJS/UMD format for the main distributable version (as rxdb does correctly), and then export an ES module as jsnext:main/module using something like Babel.

@pubkey pubkey closed this as completed in d3a14cc May 16, 2017
@pubkey
Copy link
Owner

pubkey commented May 16, 2017

The commit d3a14cc417b04e32e2c534908dc62b0bcd654a5f handles this.
It would be nice if someone could countercheck this by pulling and run npm run build to verify the build-output.

@pubkey pubkey reopened this May 16, 2017
@varbrad
Copy link
Contributor

varbrad commented May 16, 2017

A few notes;

  • cross-env should be declared as a dev-dependency, I had to install it.
  • npm run build gives me an error, seems like an async function within the scripts/transpile.js file is tripping up NodeJS.
  • npm run build:es works, and seems to look better (no async stuff), but UglifyJS still blows up in my face with some errors.

Not sure if this is an option, but could the 'ES' module version just not be uglified? Looking at most other modules such as vue, redux et al, they don't uglify their ES module builds.

Plus, can UglifyJS even handle the export ES syntax?

@pubkey
Copy link
Owner

pubkey commented May 16, 2017

@varbrad Thank you for testing.
I added cross-env to the dev-dependencies.
Which node-version do you have? You need sth like 7.x.x for the build-script.

The es-version is not uglified. RxDB does not use UglifyJS in any build-context. Everything is just transpiled.
EDIT: I'm sorry, I think you mean rxdb.browserify.min.js which is of course minified. I'm aware of the warnings here.

I don't know if UglifyJS can handle export.
But without export/import, what is then the purpose of jsnext:main/module?

@pubkey
Copy link
Owner

pubkey commented May 16, 2017

@lyquocnam I created a branch where I commited a new build.
Could you add rxdb to your package.json by the following commit-hash:

rxdb: "git+https://github.com/pubkey/rxdb/commit/d3d70e919f10d7f5178f6febeb6707ed089c8100"

Then re-run npm install and check if the build works now.

@varbrad
Copy link
Contributor

varbrad commented May 16, 2017

Ah that's why I get an error, I was running under my default node version (6.10), it work's fine on 7.10.

And yes sorry about the Uglify confusion, I was being stupid and just directly cloning the newer commit into my testing project's node_modules, forgetting that I am the one who is uglifying the source, not RxDB 😆 . I think all is good now, but I will test that latest commit again without trying to directly uglify the source this time 👍 .

@varbrad
Copy link
Contributor

varbrad commented May 16, 2017

I can confirm the latest commit d3d70e9 does indeed fix this issue. Awesome 👍. Just make sure to use the correct babel preset @lyquocnam, ES2015 won't work (you will also need stage-0 or just use env)

@pubkey
Copy link
Owner

pubkey commented May 16, 2017

@varbrad Great. I will do a new release tomorrow evening. (now +20 hours)

@lyquocnam
Copy link
Author

lyquocnam commented May 16, 2017

@pubkey i tried but error:

PS F:\dev\myapp> npm install
npm WARN addRemoteGit Error: Command failed: git -c core.longpaths=true config --get remote.origin.url
npm WARN addRemoteGit
npm WARN addRemoteGit     at ChildProcess.exithandler (child_process.js:205:12)
npm WARN addRemoteGit     at emitTwo (events.js:106:13)
npm WARN addRemoteGit     at ChildProcess.emit (events.js:194:7)
npm WARN addRemoteGit     at maybeClose (internal/child_process.js:899:16)
npm WARN addRemoteGit     at Socket.<anonymous> (internal/child_process.js:342:11)
npm WARN addRemoteGit     at emitOne (events.js:96:13)
npm WARN addRemoteGit     at Socket.emit (events.js:191:7)
npm WARN addRemoteGit     at Pipe._handle.close [as _onclose] (net.js:510:12)
npm WARN addRemoteGit  git+https://github.com/pubkey/rxdb/commit/d3d70e919f10d7f5178f6febeb6707ed089c8100 resetting remote C:\Users\lyquo\AppData\Roaming\npm-cache\_git-remotes\git-https-github-com-pubkey-rxdb-commit-d3d70e919f10d7f5178f6febeb6707ed089c8100-23a1eab8 because of error: { Error: Command failed: git -c core.longpaths=true config --get remote.origin.url
npm WARN addRemoteGit
npm WARN addRemoteGit     at ChildProcess.exithandler (child_process.js:205:12)
npm WARN addRemoteGit     at emitTwo (events.js:106:13)
npm WARN addRemoteGit     at ChildProcess.emit (events.js:194:7)
npm WARN addRemoteGit     at maybeClose (internal/child_process.js:899:16)
npm WARN addRemoteGit     at Socket.<anonymous> (internal/child_process.js:342:11)
npm WARN addRemoteGit     at emitOne (events.js:96:13)
npm WARN addRemoteGit     at Socket.emit (events.js:191:7)
npm WARN addRemoteGit     at Pipe._handle.close [as _onclose] (net.js:510:12)
npm WARN addRemoteGit   killed: false,
npm WARN addRemoteGit   code: 1,
npm WARN addRemoteGit   signal: null,
npm WARN addRemoteGit   cmd: 'git -c core.longpaths=true config --get remote.origin.url' }
npm ERR! git clone --template=C:\Users\lyquo\AppData\Roaming\npm-cache\_git-remotes\_templates --mirror https://github.com/pubkey/rxdb/commit/d3d70e919f10d7f5178f6febeb6707ed089c8100 C:\Users\lyquo\AppData\Roaming\npm-cache\_git-remotes\git-https-github-com-pubkey-rxdb-commit-d3d70e919f10d7f5178f6febeb6707ed089c8100-23a1eab8: Cloning into bare repository 'C:\Users\lyquo\AppData\Roaming\npm-cache\_git-remotes\git-https-github-com-pubkey-rxdb-commit-d3d70e919f10d7f5178f6febeb6707ed089c8100-23a1eab8'...
npm ERR! git clone --template=C:\Users\lyquo\AppData\Roaming\npm-cache\_git-remotes\_templates --mirror https://github.com/pubkey/rxdb/commit/d3d70e919f10d7f5178f6febeb6707ed089c8100 C:\Users\lyquo\AppData\Roaming\npm-cache\_git-remotes\git-https-github-com-pubkey-rxdb-commit-d3d70e919f10d7f5178f6febeb6707ed089c8100-23a1eab8: remote: Not Found
npm ERR! git clone --template=C:\Users\lyquo\AppData\Roaming\npm-cache\_git-remotes\_templates --mirror https://github.com/pubkey/rxdb/commit/d3d70e919f10d7f5178f6febeb6707ed089c8100 C:\Users\lyquo\AppData\Roaming\npm-cache\_git-remotes\git-https-github-com-pubkey-rxdb-commit-d3d70e919f10d7f5178f6febeb6707ed089c8100-23a1eab8: fatal: repository 'https://github.com/pubkey/rxdb/commit/d3d70e919f10d7f5178f6febeb6707ed089c8100/' not found
npm ERR! Windows_NT 10.0.15063
npm ERR! argv "F:\\Program Files\\nodejs\\node.exe" "F:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js" "install"
npm ERR! node v7.9.0
npm ERR! npm  v4.2.0
npm ERR! code 128

npm ERR! Command failed: git -c core.longpaths=true clone --template=C:\Users\lyquo\AppData\Roaming\npm-cache\_git-remotes\_templates --mirror https://github.com/pubkey/rxdb/commit/d3d70e919f10d7f5178f6febeb6707ed089c8100 C:\Users\lyquo\AppData\Roaming\npm-cache\_git-remotes\git-https-github-com-pubkey-rxdb-commit-d3d70e919f10d7f5178f6febeb6707ed089c8100-23a1eab8
npm ERR! Cloning into bare repository 'C:\Users\lyquo\AppData\Roaming\npm-cache\_git-remotes\git-https-github-com-pubkey-rxdb-commit-d3d70e919f10d7f5178f6febeb6707ed089c8100-23a1eab8'...
npm ERR! remote: Not Found
npm ERR! fatal: repository 'https://github.com/pubkey/rxdb/commit/d3d70e919f10d7f5178f6febeb6707ed089c8100/' not found
npm ERR!
npm ERR!
npm ERR! If you need help, you may report this error at:
npm ERR!     <https://github.com/npm/npm/issues>

npm ERR! Please include the following file with any support request:
npm ERR!     C:\Users\lyquo\AppData\Roaming\npm-cache\_logs\2017-05-16T23_02_32_990Z-debug.log
PS F:\dev\myapp>

may wait for next release :)

@varbrad
Copy link
Contributor

varbrad commented May 16, 2017

You can also just install up to the latest master commit via npm install git://github.com/pubkey/rxdb.

@lyquocnam
Copy link
Author

lyquocnam commented May 16, 2017

@varbrad yeah, it work, the error not happen again ! 🥇
but when run my app with cordova, it not reactive when inserted record like before this fix.
May this have some problem.

...
  var sub = db.khu.find().$
          .filter(x=>x != null)
          .subscribe(results => {
            self.results = results;
  });

demo app:
rxdb report bug
Edit: demo run with cordova browser and android, with normal web, it work fine !

@pubkey
Copy link
Owner

pubkey commented May 17, 2017

@lyquocnam The tests and our examples work.
Have you enabled the QueryChangeDetector?

@lyquocnam
Copy link
Author

@pubkey you mean is this : https://github.com/pubkey/rxdb/blob/master/docs/QueryChangeDetection.md ? i tried to do the tutorial, but still not reactive like before

@pubkey
Copy link
Owner

pubkey commented May 17, 2017

I just wanted to know if you enabled it. Do not enable it.

Which adapter do you use with cordova?

@lyquocnam
Copy link
Author

lyquocnam commented May 17, 2017

@pubkey i use websql, cordova platform: android and browser

@pubkey
Copy link
Owner

pubkey commented May 17, 2017

I released v4.0.1 which solves this issue by shipping es6-code instead of es7-stage-0.

@lyquocnam For the other problem (reactive-not-working), please create a new issue with steps to reproduce.

@pubkey pubkey closed this as completed May 17, 2017
@lyquocnam
Copy link
Author

lyquocnam commented May 17, 2017

Something still error @pubkey . The error before has resolved, but this new error showing. My app still run ok, but when build it show this error:
image

code at line 5326:

/**
 * async version of assert.throws
 * @param  {function}  test
 * @param  {Error|TypeError|string} [error=Error] error
 * @param  {?string} [contains=''] contains
 * @return {Promise}       [description]
 */
let assertThrowsAsync = (() => {
    var _ref = _asyncToGenerator(function* (test, error = Error, contains = '') {
        const shouldErrorName = typeof error === 'string' ? error : error.name;

        try {
            yield test();
        } catch (e) {

            // wrong type
            if (e.constructor.name != shouldErrorName) {
                throw new Error(`
            util.assertThrowsAsync(): Wrong Error-type
            - is    : ${e.constructor.name}
            - should: ${shouldErrorName}
            - error: ${e.toString()}
            `);
            }

            // check if contains
            if (contains != '' && !e.toString().includes(contains)) {
                throw new Error(`
              util.assertThrowsAsync(): Error does not contain
              - should contain: ${contains}
              - is string: ${e.toString()}
            `);
            }
            // all is ok
            return 'util.assertThrowsAsync(): everything is fine';
        }
        throw new Error('util.assertThrowsAsync(): Missing rejection' + (error ? ' with ' + error.name : ''));
    });

    return function assertThrowsAsync(_x) {
        return _ref.apply(this, arguments);
    };
})();

@pubkey
Copy link
Owner

pubkey commented May 17, 2017

@lyquocnam I could reproduce this on the angular2-example.
I will investigate..

@pubkey pubkey reopened this May 17, 2017
@pubkey
Copy link
Owner

pubkey commented May 17, 2017

I also could reproduce the non-reactive-bug.
Looks like it does not work on the build, but on dev-mode

UPDATE: It looks like UglifyJS destroys the reactive behavior, especially when keep_fnames is set to true

@pubkey
Copy link
Owner

pubkey commented May 17, 2017

I released 4.0.2 a few seconds ago.
One problem was the usage of constructor.name=='WhatEver' which does not work after minification.
Another thing was that we still had es7-things in the es6-build which of course broke UglifyJS

@pubkey pubkey closed this as completed May 17, 2017
@lyquocnam
Copy link
Author

@pubkey thanks so much, all work perfectly ! reactive still work correct 👍

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

4 participants