Changing doc.attrs?

Ah ok. Purism is of course fine. I just wanted to make sure I wasn’t missing some practical reason for doing this.

Well it’s also about consistence: everything that’s part of the prosemirror model should be treated equally - the doc node included. And it is ! just not when it comes to positions.

1 Like

This is another thing that should change between version 1.X and 2.X. The work-around is still working, so we’d be OK with it staying this way for 1.X.

@kapouer Did you have luck adding a new step for this?

It’s still on my TODO list, though i suppose it’s going to be quite easy to do.

I’ve adapted the approach that johanneswilm describes (adding a top-level node under the doc node), but have discovered some issues with it:

  • Select-all (e.g. mod-a) and delete removes the top level node, clearing any data stored in it.
  • Delete or backspace at the correct positions in an empty document can delete the top-level node.

I’ve tried making the node unselectable in the schema, but that didn’t help.

The data I’m storing in this top-level node would be better suited to a plugin’s state, but in my case it’s conceptually part of the document, and the document is what I’m storing. i.e. I’m serializing the document node - not the entire state - for storage, and using the document change as a trigger for storing being necessary (only the document is exposed to external code). Having plugin state be at the same level as the doc and selection makes this more complex.

1 Like

To add an update: I’ve dropped the extra node approach and implemented custom Steps that modify doc.attrs - as suggested - and it is working very well.

2 Likes

Great - is it code you would want to share or are each one of us three going to do this by ourselves? :slight_smile:

Hi johanneswilm - I’m happy to share, but I haven’t implemented this in a generic way, so it’s not particularly useful verbatim. It’s pretty simple though:

class AddTodo extends Step {
  constructor(todo) {
    super();

    this.todo = todo;
  }

  apply(doc) {
    doc.attrs.todos.push(this.todo);

    return StepResult.ok(doc);
  }

  invert() {
    return new RemoveTodo(this.todo);
  }

  map(mapping) {
    return this;
  }

  toJSON() {
    return {
      stepType: "addTodo",
      todo: this.todo
    };
  }

  static fromJSON(json) {
    return new AddTodo(json.todo);
  }
}
Step.jsonID("addTodo", AddTodo);

which is used as:

state.tr.step(new AddTodo(todo));

Modifying the doc.attrs value directly seems a bit wrong, but appears to work as desired. Note that I’m only ever using this in conjunction with a step that actually modifies the document content (in a standard way), so I’m not sure how well this would work if it’s the only step being applied.

6 Likes

Thanks, @Jordan this helped

I have made a quick tweak to the above so you can set and remove individual keys. Might save other people some time.

Usage:

const transaction = state.tr
  .step(new SetDocAttr('foo', 'bar'))
  .step(new SetDocAttr('bar', ['foo', 'foo']));

dispatch(transaction)

Custom Step:

// @flow
import { Step, StepResult } from 'prosemirror-transform';

class SetDocAttr extends Step {
  constructor(key: string, value: any, stepType?: string = 'SetDocAttr') {
    super();
    this.stepType = stepType;
    this.key = key;
    this.value = value;
  }
  apply(doc) {
    this.prevValue = doc.attrs[this.key];
    doc.attrs[this.key] = this.value;
    return StepResult.ok(doc);
  }
  invert() {
    return new SetDocAttr(this.key, this.prevValue, 'revertSetDocAttr');
  }
  map() {
    return null;
  }
  toJSON() {
    return {
      stepType: this.stepType,
      key: this.key,
      value: this.value,
    };
  }
  static fromJSON(json) {
    return new SetDocAttr(json.key, json.value, json.stepType);
  }
}
5 Likes

Thank you guys, I had exactly the same problem and the solution posted really saves my day! Cheers.

Update: to avoid cloberring doc.type.defaultAttrs i had to add this check:

apply(doc) {
    this.prevValue = doc.attrs[this.key];
    if (doc.attrs == doc.type.defaultAttrs) doc.attrs = Object.assign({}, doc.attrs);
    doc.attrs[this.key] = this.value;
    return StepResult.ok(doc);
}
1 Like

Hello all,

Sorry to revive this topic but I just wanted to share some updated code, as using some of the code above I ran into some issues.

The fromJSON method in the examples above is expecting just a single json argument, but the actual signature is fromJSON(schema, json), which is pretty important if you want this to work!

I’ve also added the check @kapouer mentions above, I haven’t had time to test/verify that it’s still relevant but I think there’s no harm in including it.

I’ve also modified the map method to return this, since mapping should have no effect on doc attribute updates (there are no positions to map). Returning null (as in examples above) would mean any time this step gets mapped the changes are lost, if I’m understanding things correctly.

Here’s the code I’m using:

const { Step, StepResult } = require('prosemirror-transform')

const STEP_TYPE = 'setDocAttr'

// adapted from https://discuss.prosemirror.net/t/changing-doc-attrs/784
class SetDocAttrStep extends Step {
  constructor (key, value) {
    super()
    this.key = key
    this.value = value
  }

  get stepType () { return STEP_TYPE }

  apply (doc) {
    this.prevValue = doc.attrs[this.key]
    // avoid clobbering doc.type.defaultAttrs
    if (doc.attrs === doc.type.defaultAttrs) doc.attrs = Object.assign({}, doc.attrs)
    doc.attrs[this.key] = this.value
    return StepResult.ok(doc)
  }

  invert () {
    return new SetDocAttrStep(this.key, this.prevValue)
  }

  // position never changes so map should always return same step
  map () { return this }

  toJSON () {
    return {
      stepType: this.stepType,
      key: this.key,
      value: this.value
    }
  }

  static fromJSON (schema, json) {
    return new SetDocAttrStep(json.key, json.value)
  }

  static register () {
    try {
      Step.jsonID(STEP_TYPE, SetDocAttrStep)
    } catch (err) {
      if (err.message !== `Duplicate use of step JSON ID ${STEP_TYPE}`) throw err
    }
    return true
  }
}

module.exports = {
  SetDocAttrStep
}

I’ve added a static register method to make it easy to import and control when it gets called. That said you may want to modify it to just call Step.jsonID(STEP_TYPE, SetDocAttrStep) on require. I try to avoid executing any code on require or creating modules that end up acting as singletons unless absolutely necessary, but needs must!

So with the code above you’d use it more or less like this:

const { SetDocAttrStep } = require('./set-doc-attr-step')

SetDocAttrStep.register()

...

const tr = editorState.tr.step(new SetDocAttrStep('attributeKey', 'attributeValue'))

editorView.dispatch(tr)

Some caveats about registering the step type:

  • must be registered before any processing of steps using prosemirror-transform
  • must make absolutely sure there is only one version of prosemirror-transform in dep tree, as step types registered via Step.jsonID() are kept in an object in the file scope of prosemirror-transform/master/src/step.js, so relies on module caching to work

I considered publishing this as a module, but considering the caveats mentioned in the code above and the frequency with which this feature’s been needed as far as threads in this forum indicate, I think there’s a strong case to be made that this should be a core feature. Perhaps in a 2.x release as mentioned above if this is breaking enough to warrant so. I’d feel better about using this if there was a canonical step type for doc attribute updates to avoid having to maintain a non-standard step type for historical reasons down the line.

2 Likes

I’m relatively new to using prosemirror so I may be mistaken but I thought the document was supposed to be immutable. Wouldn’t this manipulate the current doc instance since it’s not creating a new doc?

@andrews - yes, the solutions in this thread modify the document in place. Whether that is a problem depends on what else you are doing in the transaction that uses this step (and how your application consumes transactions). We didn’t see any specific issues with this in the original situations we used this approach in, but have since switched to apply functions that produce a new document to support some additional use cases, e.g.:

apply(doc) { 
  const newDoc = Node.fromJSON(schema, doc.toJSON());

  newDoc.attrs.someData = produceNewSomeData(doc.attrs.someData);

  return StepResult.ok(newDoc);
}
1 Like

Thanks. Yes that makes sense - would probably just need to alter the example to use the newDoc in the result. :slight_smile:

1 Like

@marijn I saw you mentioned back in '18 this might be a good thing to add to prosemirror-transform. I would like to add this to ProseMirror.Net and, if it’s still a good idea, prosemirror-transform. Would you take a PR? If so what would it look like? -1 or some other option to address doc? A separate step completely?

Thanks!

It would have to be a separate step. And yeah, I’d be okay to review a PR for this.

Awesome! I’ll get that together.

I’d also like to propose an option to relax attr key restrictions… Currently only attrs with keys defined in the spec will assigned at node creation due to computeAttrs; extra keys are silently ignored.

If there were a spec option to allow undefined keys, or perhaps keys matching some pattern, then arbitrary metadata could be stored in attrs with key-level conflicts without having to use the metadata node pattern…

I could open an RFC for this… Doesn’t look like that repo sees much action.

Edit: To expand on “key-level conflicts” I mean that because we have key-level update granularity we can update dynamic keyed metadata with last-writer-wins(or a custom conflict resolution strategy via transaction interceptors) semantics per-key. Vs at the object level, or having to resort to a metadata node.

I don’t think I want to change this. A custom step similar to AttrStep could make updates to properties of some generic ‘meta’ attribute less conflicty, maybe.

Fair. Yeah, I can make a custom step that operates over an attrs object instead of attrs itself.