Allow the encoding to be set via the moduleConfig object

[mojavelinux]

Opal sets the string encoding to UTF-16LE by default. It also makes this value available via the encoding property on any string object (e.g., str.encoding). This causes certain JavaScript libraries to behave incorrectly (see https://github.com/avajs/caching-transform/pull/8).

I understand that Opal isn't likely going to change the default encoding since, technically, it's the correct default for JavaScript. However, we need some solution to this clash. So the most logical place to stick this override in the moduleConfig for Asciidoctor.

const asciidoctor = require('asciidoctor.js')({ defaultEncoding: 'UTF-8' })

This will, in turn, alter the global state of Opal.

String.prototype.encoding = Opal.const_get_local(Opal.const_get_qualified('::', 'Encoding'), 'UTF_8')

What I'm aiming to do is get rid of this hack in Antora.

https://gitlab.com/antora/antora/blob/7fb70c8859a98ba83c6b484bd430e78422bd20d6/packages/asciidoc-loader/lib/load-asciidoc.js#L3-7

[ggrossetie]

@mojavelinux Should we get it fixed in 1.5.6 ?

[mojavelinux]

Certainly not for 1.5.6-rc.1. If we see an easy solution for 1.5.6, we can move forward with it. However, if it needs some design work, then we should hold off until 1.5.7.

[ggrossetie]

Ok!

[mojavelinux]

Could we reconsider this change in 2.x? Or is this fixed by https://github.com/opal/opal/issues/2138. One way to verify is to remove the workarounds I'm applying in Antora and in the test suite.

To apply my patch, I now have to add a direct dependency on the Opal runtime in order to make this change, and that makes it difficult to support a version range for Asciidoctor.js.

[ggrossetie]

And also https://github.com/opal/opal/pull/2117 but Asciidoctor 2.2.x is not using Opal 1.1.x. If we want to include this change in 2.x we will need to backport them on Opal 0.11.

Asciidoctor.js 3.x will be based on the latest version of Opal (currently, 1.1.1) and will require Node 14.

[mojavelinux]

It would be really helpful for Antora if we could have some way of making this available to Asciidoctor.js 2. In Antora, I really want to be able to remove the direct dependency on the asciidoctor-opal-runtime so we don't get dependency mismatches, yet we still need to be able to support Node 10 as a performance escape hatch (hence we're not ready for Asciidoctor.js 3).

Is there anything I can do to help make this happen?

[ggrossetie]

@mojavelinux Sure! What we need to do:

1. Add test cases in Asciidoctor.js (in theory they should pass on https://github.com/asciidoctor/asciidoctor.js/tree/master branch but fail on https://github.com/asciidoctor/asciidoctor.js/tree/v2.2.x branch)
2. Backport/cherry-pick https://github.com/opal/opal/issues/2138 and https://github.com/opal/opal/pull/2117 to https://github.com/Mogztter/opal/tree/0.11.x-asciidoctor
3. Update https://github.com/Mogztter/asciidoctor-opal-node-runtime/tree/0.3.x (you can run the update.js script or nodejs.js, opal.js, pathname.js and stringio.js from opal build directory)
4. Cherry-pick test cases added in 1. to the https://github.com/asciidoctor/asciidoctor.js/tree/v2.2.x branch
5. Update the asciidoctor-opal-runtime dependency (install from a local path) on the https://github.com/asciidoctor/asciidoctor.js/tree/v2.2.x branch and make sure that all tests are running
6. Open pull requests
7. Release everything
8. Enjoy!

[mojavelinux]

Great! Thanks for providing those steps.

I can contribute two tests from the Antora test suite:

• https://gitlab.com/antora/antora/-/blob/master/packages/asciidoc-loader/test/load-asciidoc-test.js#L181
• https://gitlab.com/antora/antora/-/blob/master/packages/asciidoc-loader/test/load-asciidoc-test.js#L185-188

It seems like getting 1a wrapped up is the best place to start since then we know what we're aiming for. I'll begin there.

[mojavelinux]

I tried adding these two tests on the master branch and they both fail:

  describe('String encoding', () => {
    it('should use UTF-8 as the default String encoding', () => {
      expect(String('foo'.encoding)).to.equal('UTF-8')
    })

    it('should return correct bytes for String', () => {
      expect('foo'.$bytesize()).to.equal(3)
      expect('foo'.$each_byte().$to_a()).to.eql([102, 111, 111])
    })
  })

Are we expecting them to pass?

[ggrossetie]

@mojavelinux yes, Asciidoctor master is using https://github.com/Mogztter/asciidoctor-opal-node-runtime/releases/tag/v1.0.0-alpha.2 which, if I'm not mistaken, does not include opal/opal#2138 nor opal/opal#2117.

So we need to release a new version of Asciidoctor Opal Runtime 😅

[mojavelinux]

Thanks for the clarification, @Mogztter! :+1: I'll proceed accordingly.