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

Add a hasCORS boolean property to the Exchange Structure /// Webpack throws error when building in Nuxt #225

Closed
kireerik opened this issue Sep 21, 2017 · 37 comments
Assignees

Comments

@kireerik
Copy link

I think it would be great to have a hasCORS boolean property in the Exchange Structure. So we would only need to define a proxy on the client side if it is really needed.

@kroitor
Copy link
Member

kroitor commented Sep 21, 2017

Awesome idea! Thanks so much, adding it right away!

@kroitor kroitor self-assigned this Sep 21, 2017
kroitor added a commit that referenced this issue Sep 21, 2017
@kroitor
Copy link
Member

kroitor commented Sep 21, 2017

Done! The flag will be available in new version 1.7.109+. Thx for your feedback!

@kireerik
Copy link
Author

Wow, thank you for adding it so fast!

I will update sometime, however with version 1.7.110 when I start my server in production mode, than I am getting the following error:

ERROR in pages\index.86b4ca59d1fc39bf44ed.js from UglifyJs
Unexpected token: name (CCXTError) [./node_modules/ccxt/ccxt.js:51,0][pages\index.86b4ca59d1fc39bf44ed.js:4503,6]
(node:6676) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: Webpack build exited with errors

Maybe something got broken since version 1.7.1, because that works fine.

@kroitor
Copy link
Member

kroitor commented Sep 22, 2017

@kireerik we have recently dropped support for very old versions of Node (since ccxt 1.7.x+), because they are outdated and don't have async/await and other modern JS features. So, if you have Node 5 or 6, please, update to 7.6.0+.

@kireerik
Copy link
Author

Thank you for your feedback! This issue occurs with Node.js 8.5.0.

@kroitor
Copy link
Member

kroitor commented Sep 22, 2017

@kireerik ok, can you please try to reinstall the dependencies in node_modules from scratch?

rm -rf ./node_modules # VERY CAREFULLY
npm cache clean --force 
npm install

Does it still complain after that?

@kireerik
Copy link
Author

Sure, I have tried the same with Yarn (I am using it for performance reasons basically), but it doesn't make any difference.

I did some manual binary like search trying different versions and I can say that this issue was introduced with version 1.7.78 and it is working up to version 1.7.77.

@xpl
Copy link
Contributor

xpl commented Sep 22, 2017

@kireerik the problem you are experiencing is related to UglifyJS that does not understand ES2015 and modern async/await syntax out of the box. You may consider adding Babel to your build pipeline (before UglifyJS), to transpile everything down to plain old ECMAScript5.

Here's how you can do it with Webpack: babel-loader.

You can also try switching to another minifier that supports ECMAScript6 and async/await, but if you're building a browser app and want to support older browsers, it would be better if you use Babel. Hope this helps!

@kireerik
Copy link
Author

kireerik commented Sep 22, 2017

Thank you @xpl for your comment!

I think I have ES6/ES7 Transpilation included with Nuxt.js. I am also using async functions and await operators in my project it self. So I guess this issue can be related to this library.

@xpl
Copy link
Contributor

xpl commented Sep 22, 2017

@kireerik Can you please upload somewhere a minimal demo project (with Nuxt and everything set up) that demonstrates the error you experience?

@kireerik
Copy link
Author

We have a minimal example as well where the issue is reproducible too. You can request access from @cklester.

@kroitor
Copy link
Member

kroitor commented Sep 22, 2017

@kireerik unfortunately, we can't access it on gitlab due to lack of permissions. If you're in contact with @cklester, can you please ask him to let us in? He's been very helpful to us previously. We could take a look to try and help fixing this bug / issue there. Thx!

@xpl
Copy link
Contributor

xpl commented Sep 22, 2017

@cklester I've just registered as https://gitlab.com/xpl, can you give me access please?

@cklester
Copy link

@xpl Done! Thank you!

@xpl
Copy link
Contributor

xpl commented Sep 22, 2017

@cklester Unfortunately, the adonuxt-ccxt-example project is still 404 for me:

screen shot 2017-09-22 at 23 16 48

@kroitor
Copy link
Member

kroitor commented Sep 22, 2017

@kireerik , @cklester I see the same as @xpl does.

UPD. Looks like it was a cache problem with GitLab had to wait a little, and now @xpl is there.

@xpl
Copy link
Contributor

xpl commented Sep 22, 2017

@cklester @kroitor Not quite... I have now access to ccxt-adonuxt-dashboard project, but the ccxt-adonuxt-example (with a minimal error repro case) mentioned by @kireerik is still not visible to me :(

@kireerik
Copy link
Author

Yes, as I can see @xpl got access to a little bit more complex project. I can technically give access for you to the more minimalistic one if @cklester allows me that (or he can do it to).

@xpl
Copy link
Contributor

xpl commented Sep 22, 2017

@kireerik @cklester I would prefer investigating into less complex project, that would save time.

@cklester
Copy link

I've granted access for @xpl to the example repo.

@xpl
Copy link
Contributor

xpl commented Sep 22, 2017

@kireerik I have troubles with reproducing the error... The steps I have performed so far on my MacOS:

Cloned the repo and installed dependencies with yarn:

git clone https://gitlab.com/kireerik/adonuxt-ccxt-example
cd adonuxt-ccxt-example
yarn install
yarn start

Then I had trouble with port 80 (not accessible on my system), so I've changed it to 3333 in the .env file, and then the launch succeeded with no errors. Here's the webpack log:

Hash: 0cd45b74de09c7762518
Version: webpack 3.5.6
Time: 15948ms
                                  Asset     Size  Chunks                    Chunk Names
                   img/logo.ffd9a8d.png  23.8 kB          [emitted]         
    pages/index.55e5c4f36a11cffa56df.js   453 kB       0  [emitted]  [big]  pages/index
layouts/default.ddcd2640e15ad7d4f7b8.js   1.6 kB       1  [emitted]         layouts/default
    pages/about.a0d446d59f9f483b197c.js  1.32 kB       2  [emitted]         pages/about
         common.b83a2094ee680ebe0eef.js   126 kB       3  [emitted]         common
            app.408cf97076731bc09cb1.js  33.5 kB       4  [emitted]         app
       manifest.0cd45b74de09c7762518.js  1.64 kB       5  [emitted]         manifest
                               LICENSES  2.04 kB          [emitted]         
 + 9 hidden assets

WARNING in asset size limit: The following asset(s) exceed the recommended size limit (300 kB).
This can impact web performance.
Assets: 
  pages/index.55e5c4f36a11cffa56df.js (453 kB)

Seems that the pages/index.55e5c4f36a11cffa56df.js file had the CCXT code included & minified, as expected:

screen shot 2017-09-23 at 01 33 00

@xpl xpl reopened this Sep 22, 2017
@xpl xpl closed this as completed Sep 22, 2017
@xpl
Copy link
Contributor

xpl commented Sep 22, 2017

@kireerik Hmm, looks like that the example project references a previous CCXT version. Will try changing it..

UPD: yep, now I see the error. Not sure what's causing it yet, though. Seems that it doesn't like the class keyword, so need to check whether the transpilation is configured correctly... I will investigate further.

@xpl
Copy link
Contributor

xpl commented Sep 22, 2017

@kireerik I suspect that it is caused by a Webpack config where the node_modules folder is excluded from Babel, like in this issue. So will need to find where you can override this setting in AdonisJS (or Nuxt? Not sure...)

@xpl
Copy link
Contributor

xpl commented Sep 23, 2017

@kireerik OK, seems I've found a way to override the Webpack config in Nuxt: How to extend webpack config?...

@xpl
Copy link
Contributor

xpl commented Sep 23, 2017

@kireerik @cklester So here's the solution. Add the following to your config/nuxt.js file:

module.exports = {

  build: {
      extend (config, { dev, isClient }) {
        
        if (isClient) {

          config.module.rules.push ({
              test: /node_modules\/ccxt\/(.+)\.js$/,
              loader: "babel-loader",
              options: {
                babelrc: false,
                cacheDirectory: false,
                presets: ["es2015"],
                plugins: ["syntax-async-functions", "transform-regenerator"]
              }
          })
        }
      }
  },

This will add CCXT to the Babel transpilation stage. Seems that it could be a common problem across various frameworks. So if you're not the only ones who are having this problem, we will reconsider adding the Babel transpilation stage to the CCXT itself.

Hope this helps.

@kireerik
Copy link
Author

@xpl Thank you for your efforts!

I have tried the mentioned solution, but I still get the exact same error.

By the way did you made any braking changes in version 1.7.78?

@xpl
Copy link
Contributor

xpl commented Sep 25, 2017

@kireerik What is your Webpack version? BTW, after adding the previously mentioned snippet to your Nuxt config, you should see this message in the log output:

[BABEL] Note: The code generator has deoptimised the styling of "/Users/mac/adonuxt-ccxt-example/node_modules/ccxt/ccxt.js" as it exceeds the max of "500KB".

It indicates that the transpilation is set up correctly and the CCXT source is going through Babel. If you don't see the message, something's wrong.

By the way did you made any braking changes in version 1.7.78?

We stopped using Babel, since the latest Node already supports all the syntax features we need, and there's no need in transpiling, and the browser people already use Babel on their ends... But it seems that even with that, some tools are just not yet ready for ES6 modules, as they expect everything in the node_modules to be pre-compiled to ES5.

If the problem persists, we will return Babel to our build pipeline, it's not a big problem.

@kireerik
Copy link
Author

As I can see it is 3.6.0. I am not getting such a notice. So this is working for you?

I see.

@xpl
Copy link
Contributor

xpl commented Sep 25, 2017

@kireerik Yep it's working on my machine, with the adonuxt-ccxt-example project. Here's my adonuxt-ccxt-example/config/nuxt.js file:

'use strict'

const resolve = require('path').resolve

module.exports = {

  build: {
      extend (config, { dev, isClient }) {
        
        if (isClient) {

          config.module.rules.push ({
              test: /node_modules\/ccxt\/(.+)\.js$/,
              loader: "babel-loader",
              options: {
                babelrc: false,
                cacheDirectory: false,
                presets: ["es2015"],
                plugins: ["syntax-async-functions", "transform-regenerator"]
              }
          })
        } 
      }
  },

  /*
  ** Headers of the page
  */
  head: {
    title: 'Adonuxt',
    meta: [
      {
        charset: 'utf-8'
      },
      {
        name: 'viewport',
        content: 'width=device-width, initial-scale=1'
      },
      {
        hid: 'description',
        name: 'description',
        content: 'Adonuxt project'
      }
    ],
    link: [
      {
        rel: 'icon',
        type: 'image/x-icon',
        href: 'favicon.ico'
      }
    ]
  },
  /*
  ** Global CSS
  */
  css: ['~assets/css/main.css'],
  /*
  ** Customize the progress-bar color
  */
  loading: { color: '#744d82' },
  /*
  ** Point to resources
  */
  srcDir: resolve(__dirname, '..', 'resources')
}

What is the OS you use? If it's Windows, then the /node_modules\/ccxt\/(.+)\.js$/ regexp may fail because on Windows they use backward slashes. In this case, try changing it (the regexp).

@xpl
Copy link
Contributor

xpl commented Sep 25, 2017

@kireerik Have you tried that fix with the example project? If you trying it with some other project, make sure you don't override the build property somewhere later in the config.

@kireerik
Copy link
Author

I see. Yes, this occurred on Windows. Now I have tried using resolve(__dirname, '..', 'node_modules/ccxt') as recommended and it somewhat works (/node_modules[\\\/]ccxt[\\\/](.+)\.js$/ works too).

However when I visit the application in the browser I am getting the following error in both development and production mode:
ccxt.js:2952 Uncaught (in promise) ReferenceError: regeneratorRuntime is not defined at Exchange.fetchTicker

So it seems ccxt is not working on the client side this way.

@xpl
Copy link
Contributor

xpl commented Sep 25, 2017

@kireerik Hmm, seems that I forgot about the babel-polyfill. So here's the final (hopefully) fix:

module.exports = {

  build: {
      extend (config, { dev, isClient }) {
        
        if (isClient && !dev) {

          config.entry.common = ['babel-polyfill', ...config.entry.common]

          config.module.rules.push ({
              test: resolve(__dirname, '..', 'node_modules/ccxt'),
              loader: "babel-loader",
              options: {
                babelrc: false,
                cacheDirectory: false,
                presets: ["es2015"],
                plugins: ["syntax-async-functions", "transform-regenerator"]
              }
          })
        } 
      }
  },

It will add the required babel-polyfill dependency to your common scripts in the Webpack config. Let me know if it helped.

@xpl xpl changed the title Add a hasCORS boolean property to the Exchange Structure Add a hasCORS boolean property to the Exchange Structure / Issues with Uglify when using with Nuxt Sep 25, 2017
@xpl xpl changed the title Add a hasCORS boolean property to the Exchange Structure / Issues with Uglify when using with Nuxt Add a hasCORS boolean property to the Exchange Structure /// Webpack throws error when building in Nuxt Sep 25, 2017
@kireerik
Copy link
Author

Thank you! It works correctly this way.

@xpl
Copy link
Contributor

xpl commented Sep 25, 2017

@kireerik UPD: Try also changing

if (isClient) {

to:

if (isClient && !dev) {

To disable the unnessesary transpilation in the development environment.

@xpl
Copy link
Contributor

xpl commented Sep 25, 2017

@kireerik Sorry, had a mistake in my previous message (now edited). Transpilation makes build times considerably slower, so it is not a something you want in the dev mode...

@kireerik
Copy link
Author

Thank you!

@Oy744
Copy link

Oy744 commented Apr 12, 2023

Tu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants