Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

std/esm stopped working with pm2 cli options #157

Closed
dnalborczyk opened this issue Nov 17, 2017 · 18 comments
Closed

std/esm stopped working with pm2 cli options #157

dnalborczyk opened this issue Nov 17, 2017 · 18 comments
Labels

Comments

@dnalborczyk
Copy link
Contributor

we are using std/esm on a production deployment with docker and pm2 (https://github.com/Unitech/pm2). for some reason, std/esm stopped working with pm2 with version 0.14.0+ for both (fork and cluster mode), version 0.13.0 works just fine.

repro steps:

pm2 start index.js --node-args="--require @std/esm"

it throws an following exception:

import foo from './foo';
^^^^^^
SyntaxError: Unexpected token import
    at createScript (vm.js:80:10)
    at Object.runInThisContext (vm.js:139:10)
    at Module._compile (module.js:599:28)
    at Object.Module._extensions..js (module.js:646:10)
    at Module.load (module.js:554:32)
    at tryModuleLoad (module.js:497:12)
/Users/x/dev/test/index.js:1

it seems that std/esm itself is being loaded, as it seem to value the cache directory -- if it has been created prior, with a different approach, see below.

interestingly, 0.14.0+ still works when started with:

require = require('@std/esm')(module, { esm: 'js' });
require('./index');
@jdalton
Copy link
Member

jdalton commented Nov 17, 2017

Hi @dnalborczyk!

Could you create a simple repro repo for me to look at?

Update:
I've updated your earlier test repo but cannot reproduce a fail.

@dnalborczyk
Copy link
Contributor Author

dnalborczyk commented Nov 17, 2017

Hey @jdalton ,

thank you for looking into this.

I'm just about to get done with work. I'll update the above repo now to use pm2.

I also tried to debug and come up with a shorter repro (w/o pm2) - with no success. :/

edit: didn't see you forked it already, let me check it out ...

@jdalton
Copy link
Member

jdalton commented Nov 17, 2017

Heads up 👷 , I just force pushed an update to the fork.

@jdalton
Copy link
Member

jdalton commented Nov 17, 2017

On a side note:
There's something funky with @babel/register and @std/esm cache load priority.
I'm looking into that one.

Update:
I found and patched ae59e73 the @babel/register cache issue (it was a @std/esm issue).

@jdalton jdalton added bug and removed invalid labels Nov 17, 2017
@dnalborczyk
Copy link
Contributor Author

geeez, you're quick. it's hard to catch up. :D

do you think the above fix is responsible for the pm2 non-load -- or unrelated? I can pull down master and check...

@jdalton
Copy link
Member

jdalton commented Nov 17, 2017

I wasn't able to reproduce a problem with pm2 it seemed to launch (and output green logs).
If you were combining it with @babel/register then probably, ya.

@dnalborczyk
Copy link
Contributor Author

dnalborczyk commented Nov 17, 2017

oh, gotcha. it only appears that it is working.

just wanted to mention: for production we're pre-compiling with Babel (except modules), and then just require everything afterwards with std/esm. I haven't tried the combination --require std/esm and --require babel with pm2.

for some reason this script didn't work for me:

"pm": "cross-env BABEL_DISABLE_CACHE=1 pm2 delete all && pm2 start index.js --node-args=\"-r @std/esm -r @babel/register\"",

but I shortened it to:

"pm": "pm2 start index.js --node-args=\"-r @std/esm\",

or, alternatively, from the cli:

npx pm2 start index.js --node-args="-r @std/esm"

the above modified script appears to work, as one can see the online status. though when you run:

npx pm2 list;
npx show 0;

afterwards, the status changed to errored. you can access the error log on the provided path in the cli-output of show under error log path.

btw, for a minute it did appear to run, but it was after I ran your start script, and then the above (modified) pm script:

"start": "cross-env BABEL_DISABLE_CACHE=1 node -r @std/esm -r @babel/register index.js"

the start script created the caching files, and then for some reason std/esm is able to read the cached values with the pm script. if you delete the caching folder, and run the pm script (in my case the modified version), pm2 doesn't start.

I just saw that you created v0.16.1. I can give it a try and see if it fixes it. otherwise I can also update your fork?

@jdalton
Copy link
Member

jdalton commented Nov 17, 2017

Feel free to update the fork... I'll make you an admin of it. I haven't published the release so there's still time to dig into things.

@dnalborczyk
Copy link
Contributor Author

dnalborczyk commented Nov 17, 2017

cool, thanks a bunch @jdalton !!

I gotta run to a birthday party, but I'll update the fork with some instructions soon afterwards!

btw: I was about to suggest to move the .esm-cache into the node_modules/.cache folder, as babel, nyc, and others do. But when I wanted to write it up, the next version of std/esm had it changed already :D

@jdalton
Copy link
Member

jdalton commented Nov 17, 2017

Hmm interesting. I'm digging in.

Update:
I found the issue. It's pm2 being clever. I'll figure out a way to support this.

Update Update:
I have a fix... patched 2194ce5

Update Update Update:
I've traveled deep into pm2, pirates, and @std/esm and have finally come out with it working locally. I'm going to cleanup the code and then update with more patches.

Update Update Update:
Patched ced8ec1 and f547bdc.

@dnalborczyk
Copy link
Contributor Author

dnalborczyk commented Nov 22, 2017

just run the latest from master 1649402 and std/esm seems to work with pm2!! -- including in conjunction with babel!!

I made sure I tested with a clean plate: deleted .cache, pm2 kill etc.

do you still want me to update your fork?

it's essentially just something like the following:
(I used a version parameter to run through different versions of std/esm, since v0.12 and v0.13 did work with pm2)

#!/bin/bash
set -ev

VERSION=$1;

# pre-clean
rm -rf package-lock.json .esm-cache node_modules;

npm install;

npm install --save-exact @std/esm@${VERSION};

# kill all existing processes
npm run pm2 kill;

# flush logs
npm run pm2 flush;

pm2 start index.js --no-daemon --no-autorestart --node-args="-r @std/esm";
# or:
# pm2 start index.js --no-daemon --no-autorestart --node-args="-r @babel/register -r @std/esm";
{
    "name": "stdesm-esm-script",
    "scripts": {
        "pm2": "pm2"
    },
    "dependencies": {
        "pm2": "2.7.2"
    },
    "@std/esm": "js"
}

@jdalton
Copy link
Member

jdalton commented Nov 22, 2017

just run the latest from master 1649402 and std/esm seems to work with pm2!! -- including in conjunction with babel!!

Rock 🤘 !

do you still want me to update your fork?

No, I have it working now 😋

@dnalborczyk
Copy link
Contributor Author

it seems that v0.17.2 unfortunately broke the usage with pm2 again :/ v0.17.1 seems to work.

would you rather have a new bug filed in cases like that? I can also bisect and try to find the responsible commit if you like?

I'm also unsure if that is something which would needed to be fixed on the pm2's side of things ...?

@jdalton
Copy link
Member

jdalton commented Nov 27, 2017

Sorry about that @dnalborczyk!

With 0.17.2 I added scenario tests for @babel/register, ava, jest, nyc, and tsc.
I'll add one for pm2 too to ensure we don't break it again.

@dnalborczyk
Copy link
Contributor Author

cool, thanks!!

let me know if I can be of any help ...

@jdalton
Copy link
Member

jdalton commented Nov 27, 2017

@dnalborczyk I found the issue. I'll have a patch, scenario test, and bump a release today.

@dnalborczyk
Copy link
Contributor Author

saweeeet!!! thanks a bunch!!

@jdalton
Copy link
Member

jdalton commented Nov 28, 2017

Patched 734c63c; Tests dd9c573.

Update:
v0.17.3 is released 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

2 participants