First step in moving away from storeNodeReferences in favor of using Roku's app-ui API instead #150
Conversation
…et children content
…yPathToSceneKeyPath
…nvertKeyPathToSceneKeyPath, getAppUI
client/src/ECP.ts
Outdated
|
|
||
| constructor(config?: ConfigOptions) { | ||
| this.device = new RokuDevice(config); | ||
| constructor(device: RokuDevice, config?: ConfigOptions) { |
There was a problem hiding this comment.
this is a breaking change for anyone constructing an ECP instance. Is there any way to combine all of this into a single config instead? Maybe something like:
config?: ConfigOptions & { device: RokuDevice}then lift it off of the options? Not a big deal that it's included in the config downstrea, is it? Or you could remove it before setting config
There was a problem hiding this comment.
Good point. I've updated the code to be backwards compatible let me know what you think. On OnDeviceComponent I have device as a separate arg so was thinking of following the same pattern
| raspFileLines = raspFileLines.concat(this.raspFileSteps); | ||
| this.raspFileSteps = undefined; | ||
| fsExtra.writeFileSync(outputPath, raspFileLines.join('\n')); | ||
| utils.getFsExtra().writeFileSync(outputPath, raspFileLines.join('\n')); |
There was a problem hiding this comment.
What was the point of this? Can we just import fsExtra directly? Even if needing to mock it, if you do import * as fsExtra then you can mock that from anywhere.
There was a problem hiding this comment.
This is due to us using it the vscode UI and svelte failing in that case
| import { utils } from './utils'; | ||
| import * as ODC from './types/OnDeviceComponent'; | ||
| import type { AppUIResponse, AppUIResponseChild } from '.'; | ||
| import { ecp } from '.'; |
There was a problem hiding this comment.
This can cause circular references in typescript imports, so best to avoid imports from .. Can you update the import to a specific file? (should do the same for the import above this as well).
There was a problem hiding this comment.
Leaving as is based on our talk about this being a singleton only added in index
client/src/OnDeviceComponent.ts
Outdated
| }; | ||
| } | ||
|
|
||
| /** deprecated will be removed in 3.0 */ |
There was a problem hiding this comment.
These would be better served with actual jsdoc annotations.
/**
* @deprecated will be removed in 3.0
*/
This PR serves as the first step in moving away from storeNodeReferences in favor of using Roku's app-ui API instead. For context the vscode extension PR that is the primary use of this work is here: rokucommunity/vscode-brightscript-language#661. It is still to be determined if storeNodeReferences will completely go away with 3.0. At the very least the plan is to remove all of the custom code to try and access ArrayGrid children