Tab completion fails for deno.land/x/polkadot with throw_on_side_effect: Some(true)

[btwiuse]

Hi, I'm building a online repl playground called Subshell for the deno.land/x/polkadot library using deno, but found some issue with tab completion functionality in deno repl, and added a small patch to fix that.

However I'm not sure if there are better ways to do it.

Steps to reproduce:

First save the following snippet to tini.ts:

import { ApiPromise, WsProvider } from 'https://deno.land/x/polkadot@0.0.1/api/mod.ts';

async function initApi(){
  const PROVIDER = Deno.env.get("PROVIDER") ?? "wss://rpc.polkadot.io";
  const provider = new WsProvider( PROVIDER );
  return await ApiPromise.create({ provider });
}

console.log("api is initializing. Please hold on...");

const api = await initApi();

Then start a repl using tini.ts as eval-file. Pressing tab(s) on the following input doesn't show any completion:

$ deno repl --eval-file=tini.ts
api is initializing. Please hold on...
Deno 1.23.1
exit using ctrl+d or close()
> api.tx.

(The equivalent code in Node.js will give such output:)

(node)
> api.tx.
api.tx.__proto__                   api.tx.hasOwnProperty              api.tx.isPrototypeOf               api.tx.propertyIsEnumerable
api.tx.toLocaleString              api.tx.valueOf

api.tx.apply                       api.tx.arguments                   api.tx.bind                        api.tx.call
api.tx.caller                      api.tx.constructor                 api.tx.toString

api.tx.auctions                    api.tx.authorship                  api.tx.babe                        api.tx.balances
api.tx.bounties                    api.tx.childBounties               api.tx.claims                      api.tx.configuration
api.tx.council                     api.tx.crowdloan                   api.tx.democracy                   api.tx.dmp
api.tx.electionProviderMultiPhase  api.tx.grandpa                     api.tx.hrmp                        api.tx.identity
api.tx.imOnline                    api.tx.indices                     api.tx.initializer                 api.tx.length
api.tx.multisig                    api.tx.name                        api.tx.paraInclusion               api.tx.paraInherent
api.tx.paras                       api.tx.parasDisputes               api.tx.parasShared                 api.tx.phragmenElection
api.tx.preimage                    api.tx.proxy                       api.tx.registrar                   api.tx.scheduler
api.tx.session                     api.tx.slots                       api.tx.staking                     api.tx.system
api.tx.technicalCommittee          api.tx.technicalMembership         api.tx.timestamp                   api.tx.tips
api.tx.treasury                    api.tx.ump                         api.tx.utility                     api.tx.vesting
api.tx.voterList                   api.tx.xcmPallet

And after some trial and error, I have located the relevant bit in deno's source tree that causes the problem.

By changing this line from

deno/cli/tools/repl/editor.rs

Line 132 in 8d82ba7

throw_on_side_effect: Some(true),

to

          throw_on_side_effect: None,

tab completion will work again:

$ subshell repl --eval-file=tini.ts
api is initializing. Please hold on...
Subshell 0.1.2
exit using ctrl+d or close()
> api.tx.
system                      imOnline                    proxy                       paras                       length                      hasOwnProperty
scheduler                   democracy                   multisig                    initializer                 name                        __lookupGetter__
preimage                    council                     bounties                    dmp                         arguments                   __lookupSetter__
babe                        technicalCommittee          childBounties               ump                         caller                      isPrototypeOf
timestamp                   phragmenElection            tips                        hrmp                        constructor                 propertyIsEnumerable
indices                     technicalMembership         electionProviderMultiPhase  parasDisputes               apply                       valueOf
balances                    treasury                    voterList                   registrar                   bind                        toLocaleString
authorship                  claims                      configuration               slots                       call
staking                     vesting                     parasShared                 auctions                    toString
session                     utility                     paraInclusion               crowdloan                   __defineGetter__
grandpa                     identity                    paraInherent                xcmPallet                   __defineSetter__
> api.tx.

I'm wondering if we can safely disable throw_on_side_effect in deno by default, so I don't have to maintain a separate fork. Or should there be a command line switch allowing people to disable this flag?

[btwiuse]

I did a grep on the source tree, found 5 occurrences of throw_on_side_effect, and 4 of them are set to None by default

$ git grep throw_on_side_effect:
cli/cdp.rs:  pub throw_on_side_effect: Option<bool>,
cli/cdp.rs:  pub throw_on_side_effect: Option<bool>,
cli/tools/repl/editor.rs:          throw_on_side_effect: Some(true),
cli/tools/repl/session.rs:        throw_on_side_effect: None
cli/tools/repl/session.rs:          throw_on_side_effect: None,
cli/tools/repl/session.rs:        throw_on_side_effect: None
cli/tools/repl/session.rs:          throw_on_side_effect: None,

Is there any special reason why it is set to Some(true) in cli/tools/repl/editor.rs ?

[lucacasonato]

We can not disable throw_on_side_effect for REPL completions. Take this example:

const x = {
  get y() {
    /* delete a bunch of files */
    return { z: 1 }
  }
};

If you pasted this in the REPL, and then tried to auto-complete here:

> x.y.
      ^

we'd have to execute the getter of y and possibly delete a bunch of files (or start a fetch, or do something else that causes a side effect). That would be very bad, because the user did not run this code, they only typed it.

I'll thus be closing this.