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

REPL: import statement inside the Node.js REPL #48084

Closed
hemanth opened this issue May 20, 2023 · 25 comments
Closed

REPL: import statement inside the Node.js REPL #48084

hemanth opened this issue May 20, 2023 · 25 comments
Labels
feature request Issues that request new features to be added to Node.js. repl Issues and PRs related to the REPL subsystem. stale

Comments

@hemanth
Copy link
Contributor

hemanth commented May 20, 2023

What is the problem this feature will solve?

Allow import statements in REPL

What is the feature you are proposing to solve the problem?

Instead of:

> import assert from 'node:assert'
import assert from 'node:assert'
^^^^^^

Uncaught:
SyntaxError: Cannot use import statement inside the Node.js REPL, alternatively use dynamic import

Can we convert them to dynamic import and execute? Just in the REPL.

Like how top level await was supported in Chrome before TLA was an accepted TC39 proposal.

What alternatives have you considered?

At the least we change the message.

from:

Uncaught:
SyntaxError: Cannot use import statement inside the Node.js REPL, alternatively use dynamic import

to:

Uncaught:
SyntaxError: Cannot use import statement inside the Node.js REPL, alternatively use dynamic import

Instead of:
  import assert from 'node: assert'

use:
 const assert = await import('node: assert')

@hemanth hemanth added the feature request Issues that request new features to be added to Node.js. label May 20, 2023
@VoltrexKeyva VoltrexKeyva added the repl Issues and PRs related to the REPL subsystem. label May 20, 2023
@aduh95
Copy link
Contributor

aduh95 commented May 20, 2023

Static imports are valid syntax only at the top-level body of a module, so that wouldn't be suited for REPL IIUC.

Like how top level await was supported in Chrome before TLA was an accepted TC39 proposal.

It should be noted that Chrome DevTools still don't let you use static imports in its console.

At the least we change the message.

That sounds like a more reasonable proposal, would you like to submit a PR?

@hemanth
Copy link
Contributor Author

hemanth commented May 20, 2023

Sure @aduh95 I can do a PR for this.

@hemanth
Copy link
Contributor Author

hemanth commented May 20, 2023

@aduh95 Wondering if we should handle all variations of static imports and convert that to dynamic imports or just show a generic example! 🤷

@aduh95
Copy link
Contributor

aduh95 commented May 20, 2023

There are three types on static import, you don’t have to treat them all the same. If you can show the actual replacement for the user, that’s of course more valuable, but if that’s too difficult it’s ok to use a generic example.

@hemanth
Copy link
Contributor Author

hemanth commented May 20, 2023

These four types should be good?

Named import: import { export1, export2 } from "module-name";
Default import: import defaultExport from "module-name";
Namespace import: import * as name from "module-name";
Side effect import: import "module-name";

But then, do we have AST capabilities baked in the source to covert this or do we depend on a robust RE?

@aduh95
Copy link
Contributor

aduh95 commented May 20, 2023

We have acorn

@hemanth
Copy link
Contributor Author

hemanth commented May 22, 2023

Added a sample to acron explorer.

import * as name from "module-name"; gets interesting.

@hemanth
Copy link
Contributor Author

hemanth commented May 22, 2023

@aduh95 It was easier than expected with acron, not clear on how to import the dep acorn and acorn-walk in repl.js though.

const toDynamicImport = (codeLine) => {
  let dynamicImportStatement = ''
  const ast = acorn.parse(codeSnippet, { sourceType: 'module' });

  walk.ancestor(ast, {
    ImportDeclaration(node, ancestors) {
      const importedModules = node.source.value;
      let importedSpecifiers = node.specifiers.map(specifier => specifier.local.name);

      if (importedSpecifiers.length > 1) {
        importedSpecifiers = `{${importedSpecifiers.join(" ")}}`
      }
      dynamicImportStatement = `const ${importedSpecifiers} = await import('${importedModules}')`;
    },
  });
  return dynamicImportStatement;
} 

self.lines.at(-2) at repl.js#694 will give us the static import lines that the user inputted.

@aduh95
Copy link
Contributor

aduh95 commented May 22, 2023

not clear on how to import the dep acorn and acorn-walk in repl.js though.

We already import it:

node/lib/repl.js

Lines 104 to 107 in 05693ac

const {
isIdentifierStart,
isIdentifierChar,
} = require('internal/deps/acorn/acorn/dist/acorn');

@hemanth
Copy link
Contributor Author

hemanth commented May 22, 2023

Ah, no wonder, it didn't show up for me in search :D

image

@silverwind
Copy link
Contributor

silverwind commented May 25, 2023

Static imports are valid syntax only at the top-level body of a module, so that wouldn't be suited for REPL IIUC.

Can't the REPL just be started as a module body instead of current CJS body? require etc still works in modules, so I think it'd a strict superset in terms of features to run the REPL as a module.

Being able to use regular import in the REPL would be a huge QOL improvement.

@aduh95
Copy link
Contributor

aduh95 commented May 25, 2023

Modules are strict mode only, I think that'd be a not-so-nice experience for a REPL.

@silverwind
Copy link
Contributor

silverwind commented May 25, 2023

I guess strictness exceptions could be made for the REPL. It's meant as a user interface. Either run as a "lax" module or transform the import to dynamic under the hood.

@ljharb
Copy link
Member

ljharb commented May 25, 2023

Anything in the REPL that isn't the same in node code is a potential footgun, or could cause confusion. There's already a few of these things like core modules being implicitly available.

@silverwind
Copy link
Contributor

silverwind commented May 25, 2023

Anything in the REPL that isn't the same in node code is a potential footgun, or could cause confusion. There's already a few of these things like core modules being implicitly available.

Maybe the REPL could have two modes, a strict one without implicit core module globals, no .help etc, and a lax one with all of these and working import statement.

Though I don't see a use case for strict REPL. If I want strict I run a script instead.

@hemanth
Copy link
Contributor Author

hemanth commented May 26, 2023

I like @silverwind's idea. I'm happy to change the PR if we have consensus here.

@silverwind
Copy link
Contributor

Default should be non-strict to not break existing use cases. A --strict-repl could then enable the strict mode that disables all the features.

@aduh95
Copy link
Contributor

aduh95 commented May 26, 2023

Though I don't see a use case for strict REPL. If I want strict I run a script instead.

I very much agree with that, introducing a different kind of REPL seems hardly worth it.

Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Nov 24, 2023
Copy link
Contributor

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 24, 2023
@liudonghua123
Copy link
Contributor

liudonghua123 commented Feb 6, 2024

Support static import will be convenient for testing some copy/paste pieces of code.

Another alternative tool for this scene is ts-node or tsx or esno, which support static import and typescript and some other features.

@silverwind
Copy link
Contributor

Support static import will be convenient for testing some copy/paste pieces of code.

Another alternative tool for this scene is ts-node or tsx or esno, which support static import and typescript and some other features.

bun repl also has this feature. It's really only node's repl that is seemingly stuck in CJS world.

@liudonghua123
Copy link
Contributor

Support static import will be convenient for testing some copy/paste pieces of code.

Another alternative tool for this scene is ts-node or tsx or esno, which support static import and typescript and some other features.

bun repl also has this feature. It's really only node's repl that is seemingly stuck in CJS world.

yeah, I tried the latest canary windows version of bun, bun repl is still not working. I looking forward the official windows release.

@silverwind
Copy link
Contributor

silverwind commented Mar 14, 2024

I think this issue may be a duplicate of #33369, even though this one has a clearer description, so maybe close the other.

@hemanth
Copy link
Contributor Author

hemanth commented Mar 21, 2024

Fix for this was implemented and merged at #48129

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. repl Issues and PRs related to the REPL subsystem. stale
Projects
None yet
Development

No branches or pull requests

7 participants
@hemanth @ljharb @silverwind @liudonghua123 @aduh95 @VoltrexKeyva and others