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

Is Node.js 12.17.0 supported for zx as a package ? #824

Closed
Cactusbone opened this issue May 27, 2024 · 7 comments · Fixed by #827
Closed

Is Node.js 12.17.0 supported for zx as a package ? #824

Cactusbone opened this issue May 27, 2024 · 7 comments · Fixed by #827

Comments

@Cactusbone
Copy link

Hello !
I've seen the releases notes of 8.1.0 which says

Extended Node.js supported versions range: from 12.17.0 to the latest 22.x.x.

However with Node.js 12.22.12 (the one shipped with alpine 3.20, upon which docker-cli images are built), I'm getting SyntaxError: Unexpected reserved word with import {$, cd} from 'zx';

What I'm trying to achieve: I'm trying to run mocha tests (using mocha binaries), which starts and stops Docker containers, so I'm using zx as a package. And since I also need docker client, I'm using the docker-cli image, with alpine 3.20, which provides Node.js 12.22.12.

It is expected to work ? Anything I can to do work around it ?

Thanks

Expected Behavior

import {$, cd} from 'zx'

No error

Actual Behavior

import {$, cd} from 'zx'

SyntaxError: Unexpected reserved word

Steps to Reproduce

  1. Use Node.js 12.22.12 (I think all Node.js 12 > 12.17.0 exhibit the issue)
  2. simply import zx in a module

Specifications

  • zx version: 8.1.1
  • Platform: windows and docker (separately)
  • Runtime: Node.js 12.22.12
@Cactusbone Cactusbone changed the title Is Node.js 12.17.0 supported for Is Node.js 12.17.0 supported for zx as a package ? May 27, 2024
@Cactusbone
Copy link
Author

Ok, I've found a workaround for Node.js 12, by using require

import {createRequire} from 'node:module';
const require = createRequire(import.meta.url);
const {$, cd} = require('zx');

I think the top level await in built index.js is not compatible with Node.js 12, should work starting with Node.js 14.8.

@antongolub
Copy link
Collaborator

Nice catch, thanks!

antongolub added a commit to antongolub/zx that referenced this issue May 27, 2024
antonmedv added a commit that referenced this issue May 27, 2024
closes #824

Co-authored-by: Anton Medvedev <anton@medv.io>
@Cactusbone
Copy link
Author

Cactusbone commented May 27, 2024

Fix does not seem to work, I'm still getting the same error. According to documentation, main entrypoint is for Node.js <= 10

Node.js 12 does support the exports entrypoint, see https://nodejs.org/docs/latest-v12.x/api/packages.html#packages_package_entry_points

@antongolub
Copy link
Collaborator

antongolub commented May 27, 2024

What a weird thing: it supports the import api, but not top-level await.

@antongolub
Copy link
Collaborator

@Cactusbone ,

Check another patch plz: 8.1.2-dev.b3fcad2

@Cactusbone
Copy link
Author

@Cactusbone ,

Check another patch plz: 8.1.2-dev.b3fcad2

It works ! :)

@Cactusbone
Copy link
Author

Thanks a lot for the quick fixes :)

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

Successfully merging a pull request may close this issue.

2 participants