refactor: remove Node-specific APIs. #56

Symbitic · 2023-06-04T18:06:33Z

This PR is intended to replace as many Node-specific APIs with their standardized equivalents. The number one thing it does is replace EventEmitter with EventTarget. I did my best to make sure nothing is a breaking change, but I'm putting this in as a draft because I'm not certain of it yet.

I'd hoped to only change the adapters for this PR and leave changing the types and Events stuff for a later PR, but that wasn't possible since EventDispatcher depended on EventEmitter. I'm really sorry for that; if there are any changes you want me to make, I'll do my best. For now, I'm leaving Deno for another PR.

I did add support for a SIMPLEBLE_ADAPTER environment variable for if a user has more than one Bluetooth adapter and wants to select a particular one.

Thanks again for your great work on this.

Symbitic · 2023-06-08T14:13:49Z

@thegecko I added an (also unfinished) AsyncIterableIterator method requestLEDevices (I wasn't sure what to call it). If you want me to rename it or save it for a separate PR, just LMK.

thegecko · 2023-11-22T20:38:58Z

I've enabled prebuilds on this PR which pass 👍
Locally, the full build fails for me while linting the TypeScript. Minor issues, but keen these are addressed:

/Users/robmor01/Projects/webbluetooth/src/adapter/adapter.ts
  25:37  error  Strings must use singlequote  quotes
  31:42  error  Strings must use singlequote  quotes

/Users/robmor01/Projects/webbluetooth/src/bluetooth.ts
  108:34  error    Strings must use singlequote                    quotes
  423:34  warning  Forbidden non-null assertion                    @typescript-eslint/no-non-null-assertion
  461:41  warning  Unexpected any. Specify a different type        @typescript-eslint/no-explicit-any
  462:1   error    Expected indentation of 16 spaces but found 14  indent
  462:26  error    A space is required after '{'                   object-curly-spacing
  462:34  error    A space is required before '}'                  object-curly-spacing

/Users/robmor01/Projects/webbluetooth/src/characteristic.ts
  30:45  error  Strings must use singlequote  quotes

/Users/robmor01/Projects/webbluetooth/src/device.ts
  53:15  error  Strings must use singlequote  quotes

/Users/robmor01/Projects/webbluetooth/src/events.ts
   96:23  error  Expected a `const` assertion instead of a literal type annotation  @typescript-eslint/prefer-as-const
  101:28  error  Expected a `const` assertion instead of a literal type annotation  @typescript-eslint/prefer-as-const
  106:29  error  Expected a `const` assertion instead of a literal type annotation  @typescript-eslint/prefer-as-const
  111:18  error  Expected a `const` assertion instead of a literal type annotation  @typescript-eslint/prefer-as-const

/Users/robmor01/Projects/webbluetooth/src/service.ts
  34:42  error  Strings must use singlequote  quotes

✖ 15 problems (13 errors, 2 warnings)
  9 errors and 0 warnings potentially fixable with the `--fix` option.

Running the test suite yarn test yields an error immediately:

TypeError: Class constructor EventTarget cannot be invoked without 'new'
    at new BluetoothAdapter (/Users/robmor01/Projects/webbluetooth/dist/adapter/adapter.js:96:47)
    at Object.<anonymous> (/Users/robmor01/Projects/webbluetooth/dist/adapter/adapter.js:521:19)
    at Module._compile (node:internal/modules/cjs/loader:1155:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1209:10)
    at Module.load (node:internal/modules/cjs/loader:1033:32)
    at Function.Module._load (node:internal/modules/cjs/loader:868:12)
    at Module.require (node:internal/modules/cjs/loader:1057:19)
    at require (node:internal/modules/cjs/helpers:103:18)
    at Object.<anonymous> (/Users/robmor01/Projects/webbluetooth/dist/bluetooth.js:118:17)
    at Module._compile (node:internal/modules/cjs/loader:1155:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1209:10)
    at Module.load (node:internal/modules/cjs/loader:1033:32)
    at Function.Module._load (node:internal/modules/cjs/loader:868:12)
    at Module.require (node:internal/modules/cjs/loader:1057:19)
    at require (node:internal/modules/cjs/helpers:103:18)
    at Object.<anonymous> (/Users/robmor01/Projects/webbluetooth/dist/index.js:42:19)
    at Module._compile (node:internal/modules/cjs/loader:1155:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1209:10)
    at Module.load (node:internal/modules/cjs/loader:1033:32)
    at Function.Module._load (node:internal/modules/cjs/loader:868:12)
    at Module.require (node:internal/modules/cjs/loader:1057:19)
    at require (node:internal/modules/cjs/helpers:103:18)
    at Object.<anonymous> (/Users/robmor01/Projects/webbluetooth/test/bluetooth.test.js:2:19)
    at Module._compile (node:internal/modules/cjs/loader:1155:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1209:10)
    at Module.load (node:internal/modules/cjs/loader:1033:32)
    at Function.Module._load (node:internal/modules/cjs/loader:868:12)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:169:29)
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:526:24)
    at async importModuleDynamicallyWrapper (node:internal/vm/module:438:15)
    at async formattedImport (/Users/robmor01/Projects/webbluetooth/node_modules/mocha/lib/nodejs/esm-utils.js:9:14)
    at async Object.exports.requireOrImport (/Users/robmor01/Projects/webbluetooth/node_modules/mocha/lib/nodejs/esm-utils.js:42:28)
    at async Object.exports.loadFilesAsync (/Users/robmor01/Projects/webbluetooth/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
    at async singleRun (/Users/robmor01/Projects/webbluetooth/node_modules/mocha/lib/cli/run-helpers.js:125:3)
    at async Object.exports.handler (/Users/robmor01/Projects/webbluetooth/node_modules/mocha/lib/cli/run.js:370:5)

Symbitic · 2024-03-10T09:41:29Z

Finally got around to fixing this. The ESLint errors were pretty easy. I eventually figured out the problem with the error was with the tsconfig target being set to es5. Changing it to es2016 did the trick. ES5 was actually removing the classes and compiling them down to old functions.

Some other changes:

Removed DOMEvent since that could interfere with e.g. instanceof Event. I just added a _handleDisconnect method to BluetoothDevice since that was the only time evt.target was set to anything but this.
Add bubbles to a lot of event initializers since that's what the spec says.
Add ValueEventInit used by availabilitychanged

…luetooth into replace-node-specific-apis

thegecko · 2024-03-10T19:41:05Z

Thanks @Symbitic All the tests pass with physical hardware, I need to look through the code changes...

Apollon77 · 2024-03-22T16:34:10Z

@Symbitic I saw you now defined also the Advertisement data event ... did you also added code to "simulate" it being fired when someone calls watchAdvertisements on the device? Because they should be relatively straight forward, or? Most of teh data are present in the device ... and then it would be ,more standard conform here.

Symbitic · 2024-03-23T04:36:11Z

Because they should be relatively straight forward, or? Most of teh data are present in the device ... and then it would be ,more standard conform here.

@Apollon77 I have a hard time understanding what you are asking for. watchAdvertisements is not yet stable in the webbluetooth spec, so it's difficult to implement at this time.

Apollon77 · 2024-03-23T17:22:28Z

Ok sorry, I wasn't aware that this is not stable (but Chrome already supports it behind a feature flag) ... Thoight might be an option too

thegecko · 2024-04-29T09:55:08Z

@Symbitic I've revisited this PR many times with a view to merge it but keep getting quite lost. I think it's because so much has changed (e.g. it looks like files have been renamed and the adapter abstraction has been removed). I can see this isn't an easy task but is there any way you can simplify this PR and reduce the churn?

Symbitic · 2024-04-29T20:21:15Z

@Symbitic I've revisited this PR many times with a view to merge it but keep getting quite lost. I think it's because so much has changed (e.g. it looks like files have been renamed and the adapter abstraction has been removed). I can see this isn't an easy task but is there any way you can simplify this PR and reduce the churn?

I'll see what I can do. I may need to open a new PR and abandon this one. I'll definitely remove SIMPLEBLE_ADAPTER now that we have another PR for this.

thegecko · 2024-04-30T13:43:12Z

I'll see what I can do. I may need to open a new PR and abandon this one. I'll definitely remove SIMPLEBLE_ADAPTER now that we have another PR for this.

Thanks and sorry I'm not being very timely with supporting your merges

Symbitic added 3 commits June 4, 2023 10:50

refactor: remove Node-specific APIs.

5812d7f

fix: replace atoi with (supposedly) safer strtol.

2ddd2e1

feat: add AsyncIterableIterator

a90bfdd

Symbitic marked this pull request as ready for review August 23, 2023 19:14

Merge branch 'master' into replace-node-specific-apis

f6e526f

thegecko and others added 2 commits November 22, 2023 20:47

Merge branch 'master' into replace-node-specific-apis

06af213

Merge branch 'master' into replace-node-specific-apis

5e31f34

Symbitic and others added 3 commits March 10, 2024 01:45

fix: remove eslint and constructor errors

f2e5446

Merge branch 'replace-node-specific-apis' of github.com:Symbitic/webb…

7bb6208

…luetooth into replace-node-specific-apis

Merge branch 'master' into replace-node-specific-apis

bf08466

Merge branch 'master' into replace-node-specific-apis

9a2b5aa

Merge branch 'master' into replace-node-specific-apis

6e2d157

Apollon77 mentioned this pull request Mar 22, 2024

addEventHandler should accept "null" to remove event handlers as per W3C specs #177

Open

Merge branch 'master' into replace-node-specific-apis

e11b0d8

Merge branch 'master' into replace-node-specific-apis

b9759df

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: remove Node-specific APIs. #56

refactor: remove Node-specific APIs. #56

Symbitic commented Jun 4, 2023

Symbitic commented Jun 8, 2023

thegecko commented Nov 22, 2023

Symbitic commented Mar 10, 2024

thegecko commented Mar 10, 2024

Apollon77 commented Mar 22, 2024

Symbitic commented Mar 23, 2024

Apollon77 commented Mar 23, 2024

thegecko commented Apr 29, 2024

Symbitic commented Apr 29, 2024

thegecko commented Apr 30, 2024

refactor: remove Node-specific APIs. #56

Are you sure you want to change the base?

refactor: remove Node-specific APIs. #56

Conversation

Symbitic commented Jun 4, 2023

Symbitic commented Jun 8, 2023

thegecko commented Nov 22, 2023

Symbitic commented Mar 10, 2024

thegecko commented Mar 10, 2024

Apollon77 commented Mar 22, 2024

Symbitic commented Mar 23, 2024

Apollon77 commented Mar 23, 2024

thegecko commented Apr 29, 2024

Symbitic commented Apr 29, 2024

thegecko commented Apr 30, 2024