Use addEventListener instead of watch

[kenchris]

I don't recall why we invented the watch, but it feels a bit like its own thing, instead of being aligned with the platform. We could go with a startNotifications() approach like WebBluetooth.

navigator.nfc.addEventListener('exchange', message => {
  for (let record of message.records) {
    let article =/[aeio]/.test(record.data.title) ? "an" : "a";
    console.log(`${record.data.name} is ${article} ${record.data.title}`);
  }
},{
  url: document.baseURI,
  recordType: "json"
});

navigator.nfc.startNotifications();

This means that all of our values in the options dict needs to qualify as a separate handler (ie if you want to remove you need the same options dict and same function

[zolkis]

We could add a filter to watch(), i.e. options.
It would be nice if addEventListener() supported options.
Next possibility is Observables (not that it would be aligned with the platform, either).
Or just do away with options.

[kenchris]

addEventListener() does support options today :-)

https://dom.spec.whatwg.org/#dictdef-addeventlisteneroptions

[zolkis]

But not those NFC options. Is it better to extend a standard dictionary with NFC-specific properties (making it apparently aligned), or have a separate call with its own, admittedly different semantics?

[kenchris]

I think it is better extending. Creating something that looks similar but has edge cases that works differently isn't really good.

Other option would be (feel free to bikeshed names)

const filter = navigator.nfc.createReadFilter({
  url: document.baseURI,
  recordType: "json"
});

const onMessage = message => {
  for (let record of message.records) {
    let article =/[aeio]/.test(record.data.title) ? "an" : "a";
    console.log(`${record.data.name} is ${article} ${record.data.title}`);
  }
};

filter.addEventListener('exchange', onMessage, {capture: false});
filter.startNotifications();

[zolkis]

I think it is better extending.

Maybe so, but what is the general consensus or mechanism on extending standard dictionaries in the platform? Do we need to register those somewhere? (imagine multiple specs adding properties which may clash)

Creating something that looks similar but has edge cases that works differently isn't really good.

Extending a standard dictionary looks exactly like this. So it's a counter-argument.

const filter = navigator.nfc.createReadFilter({

So we'd need to specify a new NFCFilter interface that contains the startNotifications() method?
Isn't that more complex than just using a watch() method?

One way we could do it is define NfcWatchOptions and NfcPushOptions as interfaces (strip the Options from the name), add the startNotifications() method, that would return an object that has the cancel() method.

[kenchris]

Extending a standard dictionary looks exactly like this. So it's a counter-argument.

It acts exactly the same. It should honor all the options from AddEventListenerOptions

So we'd need to specify a new NFCFilter interface

Sure, and it's more similar to WebBluetooth which has getCharacteristic() which returns an object with addEventListener and startNotifications()

watch feels really alien to me

Would be really interesting hearing opinions from @domenic @reillyeon

[zolkis]

On the other hand we can't really expect to handle NFC interactions with Bluetooth interactions.

[reillyeon]

I think in general APIs are migrating away from explicitly passing callbacks in favor of either Promises for one-shot events or event listeners for repeating events. I like the idea of creating an NFCWatch (or NFCReadFilter) and adding an event listener to that. Unless it is absolutely necessary a startNotifications() method could be overkill. It is useful for Web Bluetooth because turning on notifications requires communicating with the device and so can fail in more interesting ways. addEventListener()/removeEventListener() could be sufficient for turning on and off notifications.

[zolkis]

Yes, something like this would do:

[Constructor(NFCWatchOptions options), SecureContext]
interface NFCReadFilter: EventTarget {
  readonly attribute USVString url;
  readonly attribute NFCRecordType recordType;
  readonly attribute USVString mediaType;
  readonly attribute NFCWatchMode mode;
  attribute EventHandler onread;
};

interface NFCReadEvent: Event {
  readonly attribute NFCMessage message;
};

or if we want to bind it to the navigator.nfc object, we could have a factory method instead of constructor (like @kenchris suggested).

[zolkis]

I still have the question from above:

what is the general consensus or mechanism on extending standard dictionaries in the platform? Do we need to register/link those somewhere? (imagine multiple specs adding properties which may clash)

[kenchris]

Yes, that is a fear, so we should be careful when doing so, and work with the people who specced the upstream feature.

Luckily, we also pass these specs through the TAG review which should catch most of these.

With the NFCReadFilter we are not really doing that though.

For factory function, I think that would make more sense for the NFC object if we expected to have multiple of those, and would be in line with Generic Sensors. But it is not really a sensor and we don't expect to have multiple (if so a default could be chosen by the browser, I guess, like camera)

[zolkis]

I am fine with factory methods, but if we wanted constructors it's well possible.

Filters are independent of NFC adapters, so no real need to bind to the nfc object syntactically (only in runtime: when when filters are constructed, implementations/algorithms should do the standard checks for support, secure context/permission etc and throw if there is an issue).

Even if there are multiple NFC readers, we have agreed to have an additional API in the future for enumerating/selecting them for write (currently all of them can read, and any of them is allowed to write, so if they are too close, it might happen to attempt writing a tag from two adapters, which is anyway a deployment problem and should be avoided). So at any point implementations will know how to handle the NFC adapters.

However, the API would be perhaps cleaner with the filter creation under the nfc object, since after this change it would only contain the push() (to become write()?) method. I suggest keeping the watch() name for a factory method of creating read filters, but I am fine with createReadFilter() as well.

[SecureContext]
interface NFC {
  Promise<void> push(NFCPushMessage message, optional NFCPushOptions options);
  Promise<NFCReadFilter> createReadFilter(optional NFCFilterOptions options);
};

Note that NFCPushOptions may now include an AbortSignal bound to an AbortController. The limitatation is to be able to cancel a push only in the way it was created (not like create with any and cancel only for peer push).

[domenic]

You shouldn't modify a foundational method like that of the platform for NFC-specific use cases. So the "Other option" from #152 (comment) makes more sense to me. (Although I'm unsure why you'd need to do { capture: false }, since it's not in a DOM tree anyway.)

Otherwise, I strongly agree that inventing your own event listener-like infrastructure is not good, and following the precedent set by other specs makes more sense. Especially if we already have such a strong parallel with Web Bluetooth.

or if we want to bind it to the navigator.nfc object, we could have a factory method instead of constructor (like @kenchris suggested).

Constructor is preferable if possible! I wonder why Web Bluetooth went with something else. Does creating it have to be async? Your last IDL block has it as promise-returning, and it's not clear to me why.

[reillyeon]

Constructor is preferable if possible! I wonder why Web Bluetooth went with something else. Does creating it have to be async? Your last IDL block has it as promise-returning, and it's not clear to me why.

BluetoothRemoteGATTCharacteristic does not have a constructor because you have to interrogate the device in order to determine what characteristics are available. I generally support using constructors however it becomes awkward to use when you can construct an object representing a capability that doesn't exist. Then you need a mechanism to report that the object is invalid rather than refusing to create it in the first place.

[zolkis]

Does creating it have to be async? Your last IDL block has it as promise-returning, and it's not clear to me why.

The watch() algorithm was asynchronous, but indeed the user intent to create a read filter does not need to wait until NFC is configured, so we could create the object right away. We could handle security/permissions/support errors with exceptions. We could eventually include an onerror event on the NFCReadFilter object to report NFC/read errors.

[kenchris]

I agree with @reillyeon we should just go for a version with a constructor. Maybe it is fine just calling it NFCReader

[Constructor(NFCReaderOptions options), SecureContext]
interface NFCReader : EventTarget {
  readonly attribute USVString scope;
  readonly attribute NFCRecordType recordType;
  readonly attribute USVString mediaType;
  readonly attribute NFCReaderMode mode;

  attribute EventHandler onread;
};

interface NFCReadEvent: Event {
  readonly attribute NFCMessage message;
};

interface NFCWriter {
  Promise<void> push(NFCMessageSource message, optional NFCWriterOptions options);
}

[zolkis]

The name NFCReader confused me a bit, since it's meant to be only a read filter originally.

[kenchris]

It is a reader, but it only reads a subset... If we get rid of the namespace and make these constructable (current recommendation) then we need to find good names for both.

It might also name sense to name these more specialized like NFCTagReader - that is another thing to think about

[zolkis]

We have one reader and multiple filters.

[kenchris]

Sure, in hardware, but you can create as many instances as you want, same with the writer. I don't have better suggestions. A file reader is also only reading one file :-) We could of course call it an Observer. NFCTagObserver

[reillyeon]

My interpretation of the name is that when the author wants to read from NFC tags they create an NFCReader.

There's a similar pattern in other APIs, for example, you can create multiple Accelerometer objects even though the hardware only has one accelerometer. The browser is responsible for mapping the web exposed object to the right platform object.

[littledan]

From the outside, NFCReader and NFCWriter sound like good names to me, and the EventTarget "filter" pattern makes sense. I agree with @reillyeon that instances of a class don't need to correspond 1:1 with pieces of hardware.

Thinking of a filter that outputs an EventTarget reminds me of patterns with Observables, which might be nice here from a perspective making an interface which works in Node.js as well, though WebNFC is a bit too tied to origins to make sense unchanged there. (I'm happy to see that the origin model is working out here!)

[zolkis]

I like the Observables pattern and NFCTagObserver name as it's clear and the semantics is conveyed more precisely, but if there is no confusion about the NFCReader name (meaning actually read-filters), NFCReader seems to be more aligned with the Web Platform.