Ju99ernaut / grapesjs-script-editor

Edit or attach script to selected component
MIT License
26 stars 15 forks source link

script comments cause SyntaxError on save #4

Closed abulka closed 3 years ago

abulka commented 3 years ago

Saving a script with comments e.g.

let el = this // hi

or

//let el = this

cause a console error

Uncaught SyntaxError: Unexpected end of input

in src/canvas/view/CanvasView.js:312. Easily reproducible in the grapesjs-script-editor demo (open the real browser console to view the error). The script seems to save OK but it is disconcerting.

Digging deeper it looks like grapesjs is trying to append a script object with innerHTML of:

innerHTML: "
      setTimeout(function() {
        var item = document.getElementById('i6x8e');
        if (!item) return;
        (function(){//let x=1333;}.bind(item))({})
      }, 1);"
Ju99ernaut commented 3 years ago

I think it's best to avoid comments as they seem to be placed awkwardly due to the way grapesjs attaches scripts:

setTimeout(function() {
    var item = document.getElementById('i6x8e');
    if (!item) return;
    (function(){//let x=1333;}.bind(item))({})
}, 1);

In your example it ends up commenting out the rest of the function.

abulka commented 3 years ago

Not being able to have comments in scripts is a pretty severe limitation - I'm building a user script heavy IDE, based on grapesjs and grapesjs-script-editor.

I wonder if escalating this to artf would do any good. My issues on grapesjs don't get much attention so I hesitate raising another issue there myself.

abulka commented 3 years ago

I think I've found a workaround. As long as a script has a trailing \n it will be saved and exported OK. The trailing newline ensures the }.bind(item))({}) that grapesjs adds is on a fresh line, and won't be 'commented out' by a comment in the user script. E.g. Something like

handleSave() {
  ...
  if (code.slice(-1) != '\n')
    code = code + '\n'
  ...

The proper fix would be to ensure grapesjs adds that newline in updateScript(view) in src/canvas/view/CanvasView.js. I'll investigate a bit more tomorrow.

Ju99ernaut commented 3 years ago

I think it would be better to fix this bug on the main grapesjs library, issues do take time to be attended to but he does address all issues eventually, you can also open PRs to speed up the process if you find the solution to the bug.

abulka commented 3 years ago

I think it would be better to fix this bug on the main grapesjs library, issues do take time to be attended to but he does address all issues eventually, you can also open PRs to speed up the process if you find the solution to the bug.

Ok have submitted a PR.