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

In our latest version (1.0.3) the npm/node/javascript instructions lead to a crash #145

Closed
obilodeau opened this issue Sep 26, 2017 · 19 comments

Comments

@obilodeau
Copy link
Member

Following the instructions in the README to install and run asciidoctor-reveal.js 1.0.3 with asciidoctor.js 1.5.5 leads to a crash:

$ node asciidoctor-revealjs.js presentation.adoc 

/home/olivier/src/asciidoc/test-npm/node_modules/opal-runtime/src/opal.js:4627
      throw exception;
      ^

    at singleton_class_alloc.TMP_1 [as $new] (/home/olivier/src/asciidoc/test-npm/node_modules/opal-runtime/src/opal.js:4853:15)
    at module_constructor.ːconst_missing [as $const_missing] (/home/olivier/src/asciidoc/test-npm/node_modules/opal-runtime/src/opal.js:2647:50)
    at module_constructor.ːconst_get [as $const_get] (/home/olivier/src/asciidoc/test-npm/node_modules/opal-runtime/src/opal.js:2624:19)
    at TopScope.Opal.get (/home/olivier/src/asciidoc/test-npm/node_modules/opal-runtime/src/opal.js:137:24)
    at /home/olivier/src/asciidoc/test-npm/node_modules/asciidoctor-reveal.js/dist/main.js:51:60
    at /home/olivier/src/asciidoc/test-npm/node_modules/asciidoctor-reveal.js/dist/main.js:52:7
    at Opal.modules.asciidoctor-revealjs/converter (/home/olivier/src/asciidoc/test-npm/node_modules/asciidoctor-reveal.js/dist/main.js:53:5)
    at Object.Opal.load (/home/olivier/src/asciidoc/test-npm/node_modules/opal-runtime/src/opal.js:1958:7)
    at Object_alloc.Opal.require [as $require] (/home/olivier/src/asciidoc/test-npm/node_modules/opal-runtime/src/opal.js:1982:17)
    at /home/olivier/src/asciidoc/test-npm/node_modules/asciidoctor-reveal.js/dist/main.js:62:10

@Mogztter any ideas where to look at?

I run node v8.5.0.

@ggrossetie
Copy link
Member

I think it's an incompatibility between the version of Opal used to compile / run.
See : https://github.com/asciidoctor/asciidoctor.js/blob/1.5.5/package.json

@obilodeau
Copy link
Member Author

So I should align Opal compiler version in my package.json with Asciidoctor.js'?

I'm going to try that tonight and if it fixes it issue an emergency 1.0.4 release. I might as well add a hardcoded document render task to catch these kinds of problems with Travis.

@ggrossetie
Copy link
Member

Yes a simple smoke test is a good idea.

With bestikk-opal-compiler version 0.3.0-integration7, you should use Asciidoctor.js version 1.5.6-preview.3 or 1.5.5.

@obilodeau
Copy link
Member Author

Is there a reason we are not specifying the asciidoctor.js dependency in our package.json?

@obilodeau
Copy link
Member Author

It looks like 1.5.5 was never pushed to npm repo:

$ npm show asciidoctor.js versions
 
[ '1.5.0-rc.2',
  '1.5.0',
  '1.5.1',
  '1.5.2',
  '1.5.3-preview.1',
  '1.5.3-preview.2',
  '1.5.3-preview.3',
  '1.5.3-preview.4',
  '1.5.3-preview.5',
  '1.5.3-rc.1',
  '1.5.3-rc.2',
  '1.5.3-rc.3',
  '1.5.3',
  '1.5.4-rc.1',
  '1.5.4',
  '1.5.5-1',
  '1.5.5-2',
  '1.5.5-3',
  '1.5.5-4',
  '1.5.5-5',
  '1.5.6-preview.1',
  '1.5.6-preview.2',
  '1.5.6-preview.3' ]

@obilodeau
Copy link
Member Author

Pulling in 1.5.5-5 instead of the documented 1.5.5-3 I get a different crash:

$ node ad.js 

/home/olivier/src/asciidoc/test-npm/node_modules/asciidoctor.js/dist/asciidoctor.js:22100
        } catch ($returner) { if ($returner === Opal.returner) { return $returner.$v } throw $returner; }
                                                                                       ^
PathResolver: uninitialized constant Asciidoctor::Converter::TemplateConverter::PathResolver

@ggrossetie
Copy link
Member

It looks like 1.5.5 was never pushed to npm repo:

Hmmm I'm confused... Travis did push the archive on the GitHub release page but did not publish to npm... 🤔

Pulling in 1.5.5-5 instead of the documented 1.5.5-3 I get a different crash

What version of asciidoctor-template.js are we using ?

Is there a reason we are not specifying the asciidoctor.js dependency in our package.json?

Not really, but since the Gem has a dependency on Asciidoctor, I think we should add it.

@obilodeau
Copy link
Member Author

What version of asciidoctor-template.js are we using ?

In master the dependency was dropped, I guess this was due to the fact that we compile the template into ruby. In 1.0.3 (which is the one I'm trying to fix right now) it's 1.5.5-3.

Right now, for testing, following the notes in HACKING.adoc, I use npm i --save ../asciidoctor-reveal.js but this has a different behavior than a straight installation (uses a symlink to link to package). I don't want to push test packages up and I'm not sure why I can't get the thing to work...

  • asciidoctor.js 1.5.5-2 (what's in asciidoctor-template.js' package.json 1.5.5-3)
  • asciidoctor-template.js 1.5.5-3
  • opal-compiler 0.10.1-integration2
  • opal-runtime 0.10.1-integration2
  • bestikk-opal-compiler 0.2.0

Side note: Having an explicit dependency on asciidoctor.js without installing it separately leads to the following problem which I'm not sure how to solve:

$ node ad.js 
module.js:529
    throw err;
    ^

Error: Cannot find module 'asciidoctor.js'
    at Function.Module._resolveFilename (module.js:527:15)
    at Function.Module._load (module.js:476:23)
    at Module.require (module.js:568:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/home/olivier/src/asciidoc/test-npm/ad.js:2:19)
    at Module._compile (module.js:624:30)
    at Object.Module._extensions..js (module.js:635:10)
    at Module.load (module.js:545:32)
    at tryModuleLoad (module.js:508:12)
    at Function.Module._load (module.js:500:3)

@obilodeau
Copy link
Member Author

Another attempt:

  • asciidoctor.js 1.5.5-5
  • asciidoctor-template.js 1.5.5-integration3
  • opal-compiler 0.11.0-integration6
  • opal-runtime 0.11.0-integration6
  • bestikk-opal-compiler 0.3.0-integration7

Tried a force install of opal-runtime 0.11.0-integration8 too. Didn't work:

$ node ad.js 
/home/olivier/src/asciidoc/asciidoctor-reveal.js/dist/main.js:71
  if (Opal.const_get_relative($nesting, 'RUBY_ENGINE')['$==']("opal")) {
           ^

TypeError: Opal.const_get_relative is not a function
    at /home/olivier/src/asciidoc/asciidoctor-reveal.js/dist/main.js:71:12
    at Object.<anonymous> (/home/olivier/src/asciidoc/asciidoctor-reveal.js/dist/main.js:78:3)
    at Module._compile (module.js:624:30)
    at Object.Module._extensions..js (module.js:635:10)
    at Module.load (module.js:545:32)
    at tryModuleLoad (module.js:508:12)
    at Function.Module._load (module.js:500:3)
    at Module.require (module.js:568:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/home/olivier/src/asciidoc/test-npm/ad.js:3:1)

I really don't like this black magic npm stuff...

@ggrossetie
Copy link
Member

ggrossetie commented Sep 27, 2017

Right now, for testing, following the notes in HACKING.adoc, I use npm i --save ../asciidoctor-reveal.js but this has a different behavior than a straight installation (uses a symlink to link to package). I don't want to push test packages up and I'm not sure why I can't get the thing to work...

If you do install a local copy with npm i --save ../asciidoctor-reveal.js you first need to generate the dist folder since the "main" is dist/main.js:

"main": "dist/main.js",

asciidoctor.js 1.5.5-2 (what's in asciidoctor-template.js' package.json 1.5.5-3)
asciidoctor-template.js 1.5.5-3
opal-compiler 0.10.1-integration2
opal-runtime 0.10.1-integration2
bestikk-opal-compiler 0.2.0

The following should work, Asciidoctor.js version v1.5.5-4 and below are using Opal 0.10.1. Do you have a stacktrace ?

Another attempt:
asciidoctor.js 1.5.5-5
asciidoctor-template.js 1.5.5-integration3
opal-compiler 0.11.0-integration6
opal-runtime 0.11.0-integration6
bestikk-opal-compiler 0.3.0-integration7

Did you compile Asciidoctor Reveal.js using bestikk-opal-compiler version 0.3.0-integration7 ? If you have a doubt you can look at the generated file and you should see comments with the Opal version: /* Generated by Opal ... */

Tried a force install of opal-runtime 0.11.0-integration8 too. Didn't work:

It's not necessary to do that and this version does not fully work with Asciidoctor.js.

Versioning
The main issue is that code generated with Opal 0.10 is not compatible with code generated with Opal 0.11. And since we don't follow semantic versioning in Asciidoctor.js, breaking changes can occur in patch version.

Opal runtime/compiler
For Opal compiler and runtime libraries, I'm following the version of Opal but since we need to apply some fixes we maintain an integration branch (hence the integration suffix).

Bestikk
Compiling Ruby library with Opal compiler is a bit tedious (see https://github.com/bestikk/bestikk-opal-compiler/blob/master/index.js). Basically Bestikk is here to simplify this process but "hide" the version of the Opal compiler/runtime.

Asciidoctor template
This library is tightly coupled to Asciidoctor.js so the best thing to do is to use the version defined in Asciidoctor.js' package.json

You should try the version defined here: https://github.com/asciidoctor/asciidoctor.js/blob/v1.5.5-5/package.json

  • asciidoctor.js 1.5.5-5
  • asciidoctor-template.js 1.5.5-integration1
  • opal-compiler 0.11.0-integration6
  • opal-runtime 0.11.0-integration6
  • bestikk-opal-compiler 0.3.0-integration5

We do have tests in Asciidoctor.js on the integration with Asciidoctor template.js: https://github.com/asciidoctor/asciidoctor.js/blob/v1.5.5-5/spec/node/asciidoctor.spec.js#L14

Since tests were green on Asciidoctor.js 1.5.5-5, this combinaison of versions should definitely works.

Sorry about this "version compatibility hell", I'm still trying to figure out how integrate each pieces. If you have any ideas on how to improve/simplify, please let me know 😉

I really don't like this black magic npm stuff...

Since we are mostly using strict version, we should be fine 😛 🎱
And yarn could also help to guarantee that the install will work exactly the same way.

@obilodeau
Copy link
Member Author

you first need to generate the dist folder

This is with npm run build inside the asciidoctor-reveal.js folder, correct?

Do you have a stacktrace ?

The original stack trace I posted:


/home/olivier/src/asciidoc/test-npm/node_modules/opal-runtime/src/opal.js:4627
      throw exception;
      ^

    at singleton_class_alloc.TMP_1 [as $new] (/home/olivier/src/asciidoc/test-npm/node_modules/opal-runtime/src/opal.js:4853:15)
    at module_constructor.ːconst_missing [as $const_missing] (/home/olivier/src/asciidoc/test-npm/node_modules/opal-runtime/src/opal.js:2647:50)
    at module_constructor.ːconst_get [as $const_get] (/home/olivier/src/asciidoc/test-npm/node_modules/opal-runtime/src/opal.js:2624:19)
    at TopScope.Opal.get (/home/olivier/src/asciidoc/test-npm/node_modules/opal-runtime/src/opal.js:137:24)
    at /home/olivier/src/asciidoc/test-npm/node_modules/asciidoctor-reveal.js/dist/main.js:51:60
    at /home/olivier/src/asciidoc/test-npm/node_modules/asciidoctor-reveal.js/dist/main.js:52:7
    at Opal.modules.asciidoctor-revealjs/converter (/home/olivier/src/asciidoc/test-npm/node_modules/asciidoctor-reveal.js/dist/main.js:53:5)
    at Object.Opal.load (/home/olivier/src/asciidoc/test-npm/node_modules/opal-runtime/src/opal.js:1958:7)
    at Object_alloc.Opal.require [as $require] (/home/olivier/src/asciidoc/test-npm/node_modules/opal-runtime/src/opal.js:1982:17)
    at /home/olivier/src/asciidoc/test-npm/node_modules/asciidoctor-reveal.js/dist/main.js:62:10

I'll give it a shot with the versions you suggested and I'll report back.

@obilodeau
Copy link
Member Author

That combination worked! Hurray!

@obilodeau
Copy link
Member Author

@obilodeau
Copy link
Member Author

@Mogztter have a look at the doc I wrote and tell me if it makes sense: https://github.com/asciidoctor/asciidoctor-reveal.js/blob/master/HACKING.adoc#node-package

@ggrossetie
Copy link
Member

Yes it's fine, thanks for your work! 👍

On master branch, I think we shouldn't mention asciidoctor-template.js since this dependency is not required anymore. What do you think ?

@obilodeau
Copy link
Member Author

Good point. Change made in 250935a.

@ggrossetie
Copy link
Member

Perfect 👌

@timgilbert
Copy link

timgilbert commented Apr 13, 2018

EDIT: so sorry for the noise - I messed around with this some more and discovered that it was an error in the slide I was trying to render. Once I excised the troublesome parts from the .adoc file, everything worked well. Please disregard this comment.

I'm following the instructions in the README and I'm also getting an error out of opal:

% node ./asciidoctor-revealjs.js
/Users/tim/.../node_modules/opal-runtime/src/opal.js:4993
      throw exception;
      ^
gsub: undefined method `gsub' for nil

I'm running node 9.11.1 and npm 5.8.0 via homebrew on OS/X. package.json has asciidoctor at "^1.1.3" and package-lock shows opal-runtime at "0.11.0-integration8".

Am I missing something? I read the above thread but didn't see anything I'm obviously doing wrong. I'm not trying to build asciidoctor or this backend locally, I just want to use them to render my slides.

@ggrossetie
Copy link
Member

No worries @timgilbert
Could you please share your "error" ? The error message is really unclear and I want to know if I can improve it.

Thanks

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

No branches or pull requests

3 participants