search

asciidoctor / asciidoctor-reveal.js

:crystal_ball: A reveal.js converter for Asciidoctor and Asciidoctor.js. Write your slides in AsciiDoc!
http://asciidoctor.org
Other
289 stars 188 forks source link

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

Closed obilodeau closed 6 years ago

obilodeau commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

obilodeau commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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...

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 commented 6 years ago

Another attempt:

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 commented 6 years ago

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:

https://github.com/asciidoctor/asciidoctor-reveal.js/blob/98ccfcb4a0d086d7394422f43a0699cfd521fec1/package.json#L5

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

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 commented 6 years ago

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 commented 6 years ago

That combination worked! Hurray!

obilodeau commented 6 years ago

Fixed with v.1.0.4 release: https://github.com/asciidoctor/asciidoctor-reveal.js/releases/tag/v1.0.4

obilodeau commented 6 years ago

@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 commented 6 years ago

Yes it's fine, thanks for your work! :+1:

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

obilodeau commented 6 years ago

Good point. Change made in 250935a.

ggrossetie commented 6 years ago

Perfect 👌

timgilbert commented 6 years ago

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 commented 6 years ago

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