pubkey / rxdb

A fast, local first, reactive Database for JavaScript Applications https://rxdb.info/
https://rxdb.info/
Apache License 2.0
21.37k stars 1.04k forks source link

Unexpected token: keyword (function) UglifyJs #172

Closed lyquocnam closed 7 years ago

lyquocnam commented 7 years ago

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: https://github.com/quasarframework/quasar/issues/573 this error make app can not run on mobile cordova, please help !

pubkey commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

rstoenescu commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 :) https://github.com/reactjs/redux/issues/1042#issuecomment-157129818

varbrad commented 7 years ago

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 commented 7 years ago

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

pubkey commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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.

varbrad commented 7 years ago

A few notes;

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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

I can confirm the latest commit d3d70e919f10d7f5178f6febeb6707ed089c8100 does indeed fix this issue. Awesome :+1:. 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 commented 7 years ago

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

lyquocnam commented 7 years ago

@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 commented 7 years ago

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

lyquocnam commented 7 years ago

@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 commented 7 years ago

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

lyquocnam commented 7 years ago

@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 commented 7 years ago

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

Which adapter do you use with cordova?

lyquocnam commented 7 years ago

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

pubkey commented 7 years ago

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.

lyquocnam commented 7 years ago

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 commented 7 years ago

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

pubkey commented 7 years ago

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 commented 7 years ago

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

lyquocnam commented 7 years ago

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