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

Block scoped declarations (let, const, function,class) not yet supported outside strict mode #275

Closed
bralat opened this issue Feb 5, 2017 · 34 comments
Labels

Comments

@bralat
Copy link

bralat commented Feb 5, 2017

Bug Report

I have read:

I am using the latest version of the library.
v 0.26.0

Expected Behavior

I was trying to run the bot after writing some lines of code. I expected that after runningnode index.js,
there will be no errors and when i send a request from telegram, get the desired result.

Actual Behavior

what happened instead was this error in the console

class TelegramBot extends EventEmitter {
^^^^^

SyntaxError: Block-scoped declarations (let, const, function, class) not yet supported outside strict mode
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:373:25)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Module.require (module.js:353:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (E:\Code\dobo\node_modules\node-telegram-bot-api\index.js:12:20)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)

So i run this instead node --use_strict index.js and got different set of errors

constructor(token, options = {}) {
                             ^

SyntaxError: Unexpected token =
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:373:25)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Module.require (module.js:353:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (E:\Code\dobo\node_modules\node-telegram-bot-api\index.js:12:20)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)

So after a while, I went through telegram.js and telegramPolling.js and changed the following function parameters

  1. options = {} to options
  2. forms = {} to forms
  3. fileOpts = {} to fileOpts

and in some cases, initialized the variable to an empty object within the function. For example

startPolling(options) {
    options = {};
    if (this.hasOpenWebHook()) {
      return Promise.reject(new Error('Polling and WebHook are mutually exclusive'));
    }
    options.restart = typeof options.restart === 'undefined' ? true : options.restart;
    if (!this._polling) {
      this._polling = new TelegramBotPolling(this._request.bind(this), this.options.polling, this.processUpdate.bind(this));
    }
    return this._polling.start(options);
  }

It was after taking these steps that it finally worked

Steps to reproduce the Behavior

  1. write an index.js file with following code
var token = '<insert token here>';

var Bot = require('node-telegram-bot-api'),
    bot = new Bot(token, { polling: true });

bot.onText(/^\/say_hello (.+)$/, function (msg, match) {
  var name = match[1];
  bot.sendMessage(msg.chat.id, 'Hello ' + name + '!').then(function () {
    // reply sent!
  });
});
  1. run node index.js

Question

My project, as at now, only tests sending messages but I could test other features If given approval

@preco21
Copy link
Contributor

preco21 commented Feb 5, 2017

What node version are you currently using?

@bralat
Copy link
Author

bralat commented Feb 5, 2017

@preco21 node version 4.4.3

@preco21
Copy link
Contributor

preco21 commented Feb 6, 2017

@CrazyAbdul I guess some of features are not supported in node@4: https://kangax.github.io/compat-table/es6/#node4

Could you upgrade your node to version 6 or 7?

@yagop
Copy link
Owner

yagop commented Feb 6, 2017

Would be better to target a 4.x Node.js version.
For some reason, Travis tests for version 4.x are passing.
What do you think @GochoMugo ?

@preco21
Copy link
Contributor

preco21 commented Feb 6, 2017

if (majorVersion === '0') {
const deprecate = require('depd')('node-telegram-bot-api');
deprecate('Node.js v0.12 and below will no longer be supported in the future');
module.exports = require('./lib/telegram');
} else {

Seems fallbacks in index.js only works when if the node version is 0.x.

Still have no idea for about TravisCI..

@preco21
Copy link
Contributor

preco21 commented Feb 6, 2017

Maybe this opts file is causing the problem? It is requiring babel-register internally, so I guess this is reason how it could worked even on node v4.

@GochoMugo
Copy link
Collaborator

@preco21 Thanks for looking into this issue. It seems Node v4 does not support the syntax fully.

We can have the fallback work also for Node v4.x. However, I was hoping to only deprecate Node v0.x! Do we deprecate v4.x too?

Also, we should check if it runs on v5.x, v6.x, v7.x!

@GochoMugo GochoMugo added the bug label Feb 6, 2017
@bralat
Copy link
Author

bralat commented Feb 6, 2017

@preco21 It seems the quickest solution for me is to upgrade node to version 6. I'll update this thread on how it turns within the next few hours.

@GochoMugo
Copy link
Collaborator

If you can use the latest version of Node, I would suggest you go with that. However, since we are promising our users that we support Node v4.x, we need to fix this.

@bralat
Copy link
Author

bralat commented Feb 6, 2017

@GochoMugo I could look into adding support for node v4 upwards

@GochoMugo
Copy link
Collaborator

@CrazyAbdul We could really do with some help! Thanks in advance!

@GochoMugo
Copy link
Collaborator

@CrazyAbdul But we should discuss how to achieve that, to avoid having you do so much work only to be discarded!

@preco21
Copy link
Contributor

preco21 commented Feb 6, 2017

What about simply do:

const majorVersion = process.versions.node.split('.')[0];
if (majorVersion === '0') {
  const deprecate = require('depd')('node-telegram-bot-api');
  deprecate('Node.js v0.12 and below will no longer be supported in the future');

  module.exports = require('./lib/telegram');
} else if (majorVersion === '4') {
  module.exports = require('./lib/telegram');
} else {
  module.exports = require('./src/telegram');
}

or even:

const majorVersion = process.versions.node.split('.')[0];

if (majorVersion === '0') {
  const deprecate = require('depd')('node-telegram-bot-api');
  deprecate('Node.js v0.12 and below will no longer be supported in the future');
}

module.exports = require(majorVersion === '0' || majorVersion === '4' ? './lib/telegram' : './src/telegram');

Also, you may need to fix mocha config to correct TravisCI.

@bralat
Copy link
Author

bralat commented Feb 6, 2017

@GochoMugo you earlier mentioned something about a fallback. What versions of node are those fallbacks for and how were they implemented. I'd want to learn from what the team has done so far before coming with a plan

@preco21
Copy link
Contributor

preco21 commented Feb 6, 2017

I guess you guys could consider about using babel-preset-env over static es2015 preset.

@bralat
Copy link
Author

bralat commented Feb 6, 2017

@preco21 then i can put the 2 js files I made node v4 compatible in ./src/telegram
I think that'll solve the issue

@preco21
Copy link
Contributor

preco21 commented Feb 6, 2017

@CrazyAbdul I guess it should be on index.js but not on src/. It does not need to be transpiled.

@bralat
Copy link
Author

bralat commented Feb 6, 2017

@preco21 yeah. I see that now
@GochoMugo with @preco21 code snippet and the changes i mentioned in the bug report, this can be resolved fairly quickly.
What do you think?

@GochoMugo
Copy link
Collaborator

My major issue is whether we should deprecate node v4.x, or fix the syntax to support node v4.x ? We really want to move away from babel!

@bralat
Copy link
Author

bralat commented Feb 6, 2017

@GochoMugo I came across this pdf showing stats on the various node versions

On page 18, it shows that majority of people use v4
And most of those using older versions intend to upgrade to v4
I think it's worth fixing the syntax

@bralat
Copy link
Author

bralat commented Feb 6, 2017

@GochoMugo what alternative to babel are you considering?

@GochoMugo
Copy link
Collaborator

@CrazyAbdul we are hoping to use the source as is i.e. no transpiling!!!

@yagop
Copy link
Owner

yagop commented Feb 6, 2017

According with https://nodesource.com/node-by-numbers v6 is more popular

@yagop
Copy link
Owner

yagop commented Feb 6, 2017

But supporting v4 without transpiling would be the best IMO

@bralat
Copy link
Author

bralat commented Feb 6, 2017

@yagop checked your link out and it is clearly more up to date than the pdf i talked about.
The trend suggests that the number of v4 users are dropping fast.
So maybe there could be a v4 fallback for now till we are convinced that there are very few of v4 users left before deprecating it

@GochoMugo
Copy link
Collaborator

So, I think we should push our community to using newer versions of Node.js. So we could deprecate older versions continuously. So it seems we will continue transpiling until the moment we will deprecate Node v4.x!

@bralat
Copy link
Author

bralat commented Feb 6, 2017

@GochoMugo should i go ahead and make a pull request?

@preco21
Copy link
Contributor

preco21 commented Feb 6, 2017

@CrazyAbdul Good to go.

IMO, transpiles are needed. I don't think it makes sense to simply change the codebase to the old grammar to support older version of the node.

Newer grammar gives us readability, maintainable code, such advantages.

@GochoMugo
Copy link
Collaborator

@CrazyAbdul Everything seems okay. We are waiting for a PR to fallback Node v4.x to use transpiled code. (@preco21's comment above is helpful.)

@preco21 I agree. At the rate at which Node releases newer versions, it seems unwise to downgrade our syntax to support older versions, which will be surpassed in quite a short time I presume.

@bralat bralat mentioned this issue Feb 7, 2017
2 tasks
@bralat
Copy link
Author

bralat commented Feb 7, 2017

@GochoMugo Pull request (#280) sent

@preco21
Copy link
Contributor

preco21 commented Feb 7, 2017

Great job guys!

GochoMugo added a commit that referenced this issue Feb 8, 2017
Bug:

  Node.js v4 does not support the ES6 syntax fully, thus we
  get the error:

    Block scoped declarations (let, const, function,class) not yet
    supported outside strict mode

Fix:

  * Load transpiled code
  * Deprecate support for Node.js v4.x

References:

  * Bug report: #275
  * PR: #280
  * Reported-by: @CrazyAbdul
  * PR-by: @CrazyAbdul
@GochoMugo
Copy link
Collaborator

PR #280 merged into master in commit d4a469df6b6eeae4c045faf7b6e63bdc765b0a15. Please try out the fix and report your results.

@bralat
Copy link
Author

bralat commented Feb 8, 2017

@GochoMugo @preco21 @yagop it's working now. I was able to run node index.js with no problems on v4.4.3. Didn't have to add the --use_strict option.

Thanks for letting me help out. Deeply appreciated

@bralat bralat closed this as completed Feb 8, 2017
GochoMugo added a commit that referenced this issue Feb 10, 2017
Added:

1. Add constructor options:
  * (#243) `options.polling.params` (by @GochoMugo, requested-by @sidelux)
1. Add methods:
  * (#74) *TelegramBot#removeReplyListener()* (by @githugger)
1. (#283) Add proper error handling (by @GochoMugo)
1. (#272) Add health-check endpoint (by @mironov)
  * `options.webHook.healthEndpoint`
1. (#152) Add test for TelegramBot#sendDocument() using 'fileOpts'
   param (by @evolun)
1. Document `options.webHook.host` (by @GochoMugo)
1. (#264) Add Bot API version to README (by @kamikazechaser)
1. Add examples:
  - (#271) WebHook on Heroku (by @TheBeastOfCaerbannog)
  - (#274) WebHook on Zeit Now (by @ferrari)

Changed:

1. (#147) Use *String#indexOf()*, instead of *RegExp#test()*, to
   find token in webhook request (by @AVVS)

Fixed:

* Fix bug:
  - (#275, #280) fix es6 syntax error on Node.js v4.x (by @CrazyAbdul)
  - (#276) promise.warning from `request-promise` (by @GochoMugo,
    reported-by @preco21)
  - (#281) fix handling error during polling (by @GochoMugo,
    reported-by @dimawebmaker)
  - (#284) fix error during deletion of already-set webhook, during
    polling (by @GochoMugo, reported-by @dcparga)
1. Fix links in documentation (by @Ni2c2k)
@GochoMugo
Copy link
Collaborator

Fixed in v0.27.0.

N-Agency-member pushed a commit to N-Agency-member/TelegramBot-API-Node.js- that referenced this issue Sep 3, 2024
Bug:

  Node.js v4 does not support the ES6 syntax fully, thus we
  get the error:

    Block scoped declarations (let, const, function,class) not yet
    supported outside strict mode

Fix:

  * Load transpiled code
  * Deprecate support for Node.js v4.x

References:

  * Bug report: yagop/node-telegram-bot-api#275
  * PR: yagop/node-telegram-bot-api#280
  * Reported-by: @CrazyAbdul
  * PR-by: @CrazyAbdul
passion-27 added a commit to passion-27/node-telegram-bot that referenced this issue Oct 2, 2024
Bug:

  Node.js v4 does not support the ES6 syntax fully, thus we
  get the error:

    Block scoped declarations (let, const, function,class) not yet
    supported outside strict mode

Fix:

  * Load transpiled code
  * Deprecate support for Node.js v4.x

References:

  * Bug report: yagop/node-telegram-bot-api#275
  * PR: yagop/node-telegram-bot-api#280
  * Reported-by: @CrazyAbdul
  * PR-by: @CrazyAbdul
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants