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

const instead of let in swipl-bundled.temp.js #5

Closed
smessie opened this issue Dec 27, 2022 · 8 comments · Fixed by #6
Closed

const instead of let in swipl-bundled.temp.js #5

smessie opened this issue Dec 27, 2022 · 8 comments · Fixed by #6

Comments

@smessie
Copy link
Contributor

smessie commented Dec 27, 2022

Hi, thanks for making this package :)

When trying to use it, I'm getting following error

✘ [ERROR] Cannot assign to "size" because it is a constant

    ../eye-js/node_modules/eyereasoner/dist/swipl-bundled.temp.js:2711:20:
      2711 │                     size = -1;
           ╵                     ~~~~

  The symbol "size" was declared a constant here:

    ../eye-js/node_modules/eyereasoner/dist/swipl-bundled.temp.js:2707:22:
      2707 │                 const size = parseInt(r.headers.get("content-length"));
           ╵                       ~~~~

/Users/ieben/WebstormProjects/SWIPL-WASM-Example/node_modules/esbuild/lib/main.js:1566
  let error = new Error(`${text}${summary}`);
              ^

Error: Build failed with 1 error:
../eye-js/node_modules/eyereasoner/dist/swipl-bundled.temp.js:2711:20: ERROR: Cannot assign to "size" because it is a constant
    at failureErrorWithLog (/Users/ieben/WebstormProjects/SWIPL-WASM-Example/node_modules/esbuild/lib/main.js:1566:15)
    at /Users/ieben/WebstormProjects/SWIPL-WASM-Example/node_modules/esbuild/lib/main.js:1024:28
    at runOnEndCallbacks (/Users/ieben/WebstormProjects/SWIPL-WASM-Example/node_modules/esbuild/lib/main.js:1438:61)
    at buildResponseToResult (/Users/ieben/WebstormProjects/SWIPL-WASM-Example/node_modules/esbuild/lib/main.js:1022:7)
    at /Users/ieben/WebstormProjects/SWIPL-WASM-Example/node_modules/esbuild/lib/main.js:1134:14
    at responseCallbacks.<computed> (/Users/ieben/WebstormProjects/SWIPL-WASM-Example/node_modules/esbuild/lib/main.js:671:9)
    at handleIncomingPacket (/Users/ieben/WebstormProjects/SWIPL-WASM-Example/node_modules/esbuild/lib/main.js:726:9)
    at Socket.readFromStdout (/Users/ieben/WebstormProjects/SWIPL-WASM-Example/node_modules/esbuild/lib/main.js:647:7)
    at Socket.emit (node:events:513:28)
    at addChunk (node:internal/streams/readable:324:12) {
  errors: [
    {
      detail: undefined,
      id: '',
      location: {
        column: 20,
        file: '../eye-js/node_modules/eyereasoner/dist/swipl-bundled.temp.js',
        length: 4,
        line: 2711,
        lineText: '                    size = -1;',
        namespace: '',
        suggestion: ''
      },
      notes: [
        {
          location: {
            column: 22,
            file: '../eye-js/node_modules/eyereasoner/dist/swipl-bundled.temp.js',
            length: 4,
            line: 2707,
            lineText: '                const size = parseInt(r.headers.get("content-length"));',
            namespace: '',
            suggestion: ''
          },
          text: 'The symbol "size" was declared a constant here:'
        }
      ],
      pluginName: '',
      text: 'Cannot assign to "size" because it is a constant'
    }
  ],
  warnings: []
}

Node.js v18.12.1

When I looked at that location, the error is correct, in swipl-bundled.temp.js the following code is included (formatted):

url_properties(url) {
    return fetch(url, {method: "HEAD"}).then(r => {
        if (r.status == 200) {
            const size = parseInt(r.headers.get("content-length"));
            const mods = r.headers.get("last-modified");
            const time = Date.parse(mods) || 0;
            if (!size instanceof Number) size = -1;
            return {url: r.url, status: r.status, size: size, last_modified: time / 1e3}
        } else {
            return {url: url, status: r.status}
        }
    })
}

As you can see, size is declared using a const and later on being reassigned.

I'm not sure where to report this issue, but as I'm getting this error using this package, I'm reporting it here and then we'll see where the fix should happen in the end :)

@smessie
Copy link
Contributor Author

smessie commented Dec 27, 2022

If I clone the repo and manually fix this in the bundled file, I get this package to work, so very promising!

@jeswr
Copy link
Member

jeswr commented Dec 27, 2022

Thanks for the report - the file is auto-generated from the build command in SWI-Prolog/npm-swipl-wasm#8; I am happy to accept a PR with your manual fix but it won't solve the underlying problem produced by either swipl-devel or emscripten; so I will also try and work out what the upstream cause is.

In the meantime could you provide a minimal repro for this error (or even better would be a test case included with the PR doing the manual fix) so that we don't accidentally re-introduce the error when migrating to depend on npm swipl wasm

@jeswr
Copy link
Member

jeswr commented Dec 27, 2022

Can you also test to see if you get the same error when using the node and web exports of SWIPL from https://github.com/rla/npm-swipl-wasm (as opposed to the bundled version used in eye-js).

@jeswr
Copy link
Member

jeswr commented Dec 27, 2022

Aha - looks like the real bug is here https://github.com/SWI-Prolog/swipl-devel/blob/952794b3b03e6b073e767465b884cf81a0207737/src/wasm/prolog.js#L583 - so I will create a PR there with a fix; the manual patch PR here would also be useful in the meantime.

@smessie
Copy link
Contributor Author

smessie commented Dec 28, 2022

I've made a PR #6 with the manual fix, it's pretty simple 😄
Due to upcoming exams I lack the time to also write test cases for this but once you've merged the PR and released a new version of this package, I can give you a working example where this package is used :)

@jeswr jeswr linked a pull request Dec 28, 2022 that will close this issue
@jeswr jeswr closed this as completed in #6 Dec 28, 2022
@smessie
Copy link
Contributor Author

smessie commented Dec 28, 2022

Thanks a lot for the quick actions. As promised, here is a working example of reasoning with EYE in the browser using this package (wrapped in another package @smessie/eye-js) https://reasoner.smessie.com.

@jeswr
Copy link
Member

jeswr commented Dec 29, 2022

wrapped in another package @smessie/eye-js

Happy to accept functions like that upstream here as well rather than having a bunch of micro-packages. Note that the logic you have seems to just be doing the same as

  // Load EYE into the SWIPL Module and run consule("eye.pl").
  loadEye(Module)

  // Load the the strings data and query as files data.n3 and query.n3 into the module
  Module.FS.writeFile('data.n3', data);
  Module.FS.writeFile('query.n3', query);

  // Execute main(['--nope', '--quiet', './data.n3', '--query', './query.n3']).
  queryOnce(Module, 'main', ['--nope', '--quiet', './data.n3', '--query', './query.n3']);

Noting that --nope is just returning the result of the query and removing the proof.

@jeswr
Copy link
Member

jeswr commented Dec 29, 2022

https://reasoner.smessie.com/.

Nice work!

I am observing the page is taking a while to load since it is loading all the WASM before rendering; might be worth lazy loading the eye-js module or; using the browser version of SWIPL in https://github.com/rla/npm-swipl-wasm/ rather than the default bundle we export as that loads the WASM file separately using a fetch call.

The SWIPL bundle currently exported here is designed to "just work" out of the on both node and browser. But that simplicity is coming at a performance cost in your case. (see SWI-Prolog/npm-swipl-wasm#1 for more info)

This was referenced Dec 29, 2022
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