Add registerDefaultEventName

[grncdr]

This allows configuring default events for arbitrary elements.

Closes #660

[grncdr]

@marcoroth here's a first pass at implementing this feature. One thing that tripped me up was needing to ensure that the event name is registered before stimulus attaches to the DOM and queries for elements. I guess in practice this would mean users need to call registerDefaultEventName before Application.start().

Another thing I'm unsure about, is the "global" nature of this data; registering a default event name will effect all Application instances.

Given that Stimulus already has a Schema concept for extending keymappings and attribute naming conventions, I've also created a second PR (#662) that moves the defaultEventNames lookup into the schema. I don't have a strong preference for either approach.

[adrienpoly]

LGTM thanks for working on this

It would be nice to have a bit of documentation. The example with shoelace component is great

[grncdr]

Hi @adrienpoly, could you clarify if you prefer this PR to the approach in #662? Coming back to this now after a few months, I feel that #662 is a better design.

I'm happy to update the docs, just want to highlight the other option here.

[marcoroth]

I think I prefer this solution here over the solution proposed in #662. In #622 you need to override the schema and need to make sure that you still handle the default events for the elements.

With the solution proposed in this PR you can call registerDefaultEventName() for any element you like to add but don't need to think about the defaults already configured in Stimulus.

I guess the other solution would be to combine both approaches. So that you can call registerDefaultEventName() on the schema instance. That way it doesn't apply to all application instances since it's just scoped to each application/schema.

[grncdr]

In #662 you need to override the schema and need to make sure that you still handle the default events for the elements.

The very first version of #662 was based on the way custom keyMappings are defined and looked like this:

stimulus/src/tests/modules/core/action_custom_default_event_tests.ts

Lines 7 to 10 in 83e84b7

	schema: Schema = {
	...defaultSchema,
	defaultEventNames: { ...defaultSchema.defaultEventNames, "some-element": () => "click" },
	}

I guess the other solution would be to combine both approaches. So that you can call registerDefaultEventName() on the schema instance. That way it doesn't apply to all application instances since it's just scoped to each application/schema.

(emphasis mine)

The problem here is that (by default) all applications share a single defaultSchema instance.

This makes schema.registerDefaultEventName() worse than a global registerDefaultEventName function IMO, because it looks local to the application but would modify the shared defaultSchema. I suspect this is why there isn't a more ergonomic schema.registerKeyMapping() API, because it would be far too easy to accidentally leak changes between application instances. Telling users to write nested object spreads is ugly, but avoids mutating shared global state.

---

What would you think of changing the default value of the schema argument to Application.constructor to do a deep copy of the defaultSchema? That would allow adding mutating methods the schema associated with an application without risking leaks between applications. The mutating methods could even be defined on Application itself, so that the Schema remains a mostly internal implementation detail.

Concretely, that could look like:

const app = new Application()

app.registerDefaultEventName('some-element', 'click')
app.registerKeyMapping('at', '@')

app.start()

and inside Application:

class Application {
  ...
  
  registerDefaultEventName(tagName: string, eventName: string | ((el: Element) => string)) {
    // mutating is fine here, because application uses it's own copy of the default schema
    if (typeof eventName === 'string') {
      this.schema.defaultEventNames[tagName] = () => eventName
    } else {
      this.schema.defaultEventNames[tagName] = eventName
    }
  }
  
  registerKeyMapping(alias: string, key: string) {
    // same here, much easier to document, and easier for users
    this.schema.keyMappings[alias] = key
  }
  
  ...
}

[marcoroth]

What would you think of changing the default value of the schema argument to Application.constructor to do a deep copy of the defaultSchema?

Oh right, I didn't think that far tbh, I kinda just assumed that's what we are already doing right now. And that every application instance already copies the defaultSchema.

We definitely shouldn't modify the global defaultSchema object. That's why I think it makes sense to copy the schema whenever a new application instance gets instantiated, as you pointed out 👍🏼

I also like your proposal of having registerDefaultEventName(...) on application itself. This also lines up with what we have with registerActionOption() already.

For now I would just focus on registerDefaultEventName in this pull request, we can look into adding registerKeyMapping in a separate pull request.