decaporg / decap-cms

A Git-based CMS for Static Site Generators
https://decapcms.org
MIT License
17.65k stars 3.02k forks source link

Potential Fix for #7152 (Slate crashing) #7153

Closed DavidWells closed 3 months ago

DavidWells commented 3 months ago

Potential fix for https://github.com/decaporg/decap-cms/issues/7152

demshy commented 3 months ago

This is a nice find @DavidWells! We also have a similar crash with Lists integration but that one seems a lot harder to reproduce consistently. If you also have any ideas there, that would be so great! Here is an issue https://github.com/decaporg/decap-cms/issues/7047 but I find it easiest to reproduce by adding a ton of list items and then manically selecting and deleting chunks of them.

There is one slight issue after your fix - the editor seems loses focus after backspace. I will look into handling this in the keyDown event of the inline "plugins" and commit into this PR if you don't mind. Might also solve the cypress test not passing.

demshy commented 3 months ago

I might be on to something here. Seems like after backspace in this scenario, the selected node is the child text of break node instead of the text node after it: Before: Screenshot 2024-03-20 at 12 07 32 selected path here is [0,4]

after backspace Screenshot 2024-03-20 at 12 07 54 selected path here is [0,1,0], but it should be [0,2]

demshy commented 3 months ago

Could do something like this to fix selection, unless we figure out why this happens in the first place..

    const [parent, at] = SlateEditor.parent(editor, editor.selection.focus.path)
    if (parent && parent.type == 'break') {
      const [, path] = SlateEditor.next(editor, { at })
      Transforms.select(editor, { path, offset: 0 })
    }

Not sure yet where to put it though, currently I put it in the handleChange function and it makes it work.

If you're in a hurry we can update the cypress tests and push this through and I will work on fixing the selection in another PR or even wait until we potentially move to Plate.

DavidWells commented 3 months ago

Not in a rush. I have a silly lil postinstall script to patch the package right now.

Haven't seen the crash since I did this. It could possibly still happen on elements with no children refs tho like table stuff.

const path = require('path')
const fs = require('fs').promises

/**
 * Asynchronously finds and replaces text in a file.
 * @param {string} filePath - The path to the file.
 * @param {RegExp} searchString - The regular expression to search for.
 * @param {string} replacementString - The string to replace the matches.
 * @returns {Promise<void>} A promise that resolves once the replacement is complete.
 */
async function findAndReplace(filePath, searchString, replacementString) {
  console.log(`Patching CMS file:`, filePath)
  try {
    // Read file content
    let data = await fs.readFile(filePath, 'utf8')

    // Perform replacement
    data = data.replace(searchString, replacementString)

    // Write updated content back to the file
    await fs.writeFile(filePath, data, 'utf8')

    console.log('Patching CMS complete')
  } catch (err) {
    console.error('Error:', err)
  }
}

// File path and strings to search and replace
const depPath = path.resolve(__dirname, '../node_modules/decap-cms-app/dist/decap-cms-app.js')
const searchString = /function\s+G\(e\){return\(0,i\.jsx\)\("br",e\.attributes\)\}/g
const replacementString = 'function G(e){return(0,i.jsx)("div",e.attributes,e.children)}'

findAndReplace(depPath, searchString, replacementString)
demshy commented 3 months ago

Fixed the e2e tests. About losing focus, I decided it's much better to lose focus than crash the editor, so I will merge this since we are hopefully getting a new editor in the future.

demshy commented 3 months ago

released as decap-cms@3.1.5