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.

Sign up for GitHub

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

Jump to bottom

Headless xml -> Javascript in Node.js: Many Problems #2603

Closed
kbruneel opened this issue Jun 30, 2019 · 7 comments
Closed

Headless xml -> Javascript in Node.js: Many Problems #2603

kbruneel opened this issue Jun 30, 2019 · 7 comments

Comments

@kbruneel
Copy link

kbruneel commented Jun 30, 2019 •
edited

Problem statement

I'm trying to translate Blockly xml into javascript using Node.js and running into many problems. I made a very small test that shows these problems: https://github.com/kbruneel/blockly-test

Each of the problems has its own tag: https://github.com/kbruneel/blockly-test/tags

  1. ReferenceError: Element is not defined

Element is not defined in the node.js environment.

  1. Error: Trying to set block style to procedure_blocks before theme was defined via Blockly.setTheme().

This is easy to solve by just calling Blockly.setTheme(), but it seems to me that in the headless case this should not be required.

  1. ReferenceError: document is not defined

document is not defined in the node.js environment.

As you can see I managed to hack a solution for each of the problems, but these are not good solutions, e.g. The solution uses global variables and in that way mess up my test suite.

I tried changing Blockly myself, but I'm not familiar with Closure and also quickly ran into more problems when trying to use the uncompressed version in the gulp build process for blockly_node_javascript_en.js.

I'm willing to help out with these problems, but I will need some help.

Expected Behavior

It should translate the xml to javascript.

Actual Behavior

It errors.

Steps to Reproduce

  1. git clone https://github.com/kbruneel/blockly-test.git
  2. git checkout tags/1_element_is_not_defined
  3. npm i
  4. node test.js
  5. ReferenceError: Element is not defined
  6. git checkout tags/2_need_to_use_Blockly_setTheme
  7. npm i
  8. node test.js
  9. Error: Trying to set block style to procedure_blocks before theme was defined via Blockly.setTheme().
  10. git checkout tags/3_document_is_not_defined
  11. npm i
  12. node test.js
  13. ReferenceError: document is not defined
  14. git checkout tags/4_now_it_works
  15. npm i
  16. node test.js
  17. It outputs the expected javascript

Stack Traces

Operating System and Browser

Additional Information
@kbruneel
Copy link
Author

One thing I tried was this:

gulp.task('blockly_javascript_en', function() {
  var srcs = [
    'blockly_compressed.js',
    'blocks_compressed.js',
    'javascript_compressed.js',
    'msg/js/en.js'
  ];
  // Concatenate the sources, appending the module export at the bottom.
  // Override textToDomDocument, providing Node alternative to DOMParser.
  return gulp.src(srcs)
      .pipe(gulp.concat('blockly_node_javascript_en.js'))
      .pipe(insert.append(`
if (typeof DOMParser !== 'function') {
    var JSDOM = require('jsdom').JSDOM;
    var html = '<!doctype html><html><head><meta charset="utf-8"></head><body></body></html>';
    var document = new JSDOM(html, {});
    var window = document.window;
    Blockly.DOMParser = window.DOMParser;
    Blockly.Element   = window.Element;
    Blockly.document  = document;
}
if (typeof module === 'object') { module.exports = Blockly; }
if (typeof window === 'object') { window.Blockly = Blockly; }\n`))
      .pipe(gulp.dest(''));
});

So I'm defining DOMParser, Element and document in the Blockly namespace. Wherever Element is used I changed it with Blockly.Element. The same with DOMParser. These only appear once so thats easy.

document appears a lot of times so I didn't do it. It seems to me that in the headless case the use of document should just be avoided.

@kbruneel
Copy link
Author

The solution above only works for node.js. To make it work for the browser Blockly.DOMParser and Blockly.Element should also be defined in the browser environment.

Any idea where would be a good place to do this?

@NeilFraser
Copy link
Contributor

Hi Karel,

Just a quick note to say that we are actively working on this. One PR that is in flight is #2606 which removes some 50 document.createElement calls. I'm told that @alschmiedt has also made some changes over the past couple of days that help too.

The Blockly team will be out for the next few days due to a holiday that's being celebrated over here in the colonies, but we'll be back on Monday.

@kbruneel
Copy link
Author

kbruneel commented Jul 4, 2019

@NeilFraser I made a fix myself that might be interesting for you guys to look it basically implements the solution I proposed above.

https://github.com/kbruneel/blockly

I can make a pull request if you want it.

@alschmiedt
Copy link
Contributor

Error 2 should be fixed by #2605

@RoboErikG
Copy link
Contributor

@kbruneel If you've got the time feel free to make a PR. This is something we'd need to find some time to look at more closely before merging (in other words, it may be a while before we reviewed it), but having a PR up will at least give us something to point folks at as an example.

@RoboErikG
Copy link
Contributor

We also already have a tracking bug for getting Node working in #2082

@kbruneel kbruneel mentioned this issue Jul 11, 2019
Closed
@rachel-fenichel rachel-fenichel mentioned this issue Jul 15, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@NeilFraser @kbruneel @RoboErikG @alschmiedt