-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Switch to esbuild and ESM first packaging (comms) #4247
Conversation
f526a7b
to
ae99423
Compare
Closing and opening to force re-test |
e59d60b
to
ebf7909
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GordonSmith a few questions about some of the comms changes
@@ -89,7 +89,7 @@ export class Topology extends StateObject<TopologyStateEx, TopologyStateEx> impl | |||
this.set({ | |||
Services: response.ServiceList | |||
}); | |||
return this.Services; | |||
return this.Services!; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I probably just don't understand what the use of this operator is, generally. But does this not clash with the | undefined
that was added to the accessor? Implying it's potentially undefined in one place and then asserting it would never be undefined here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its asserting that this.Services
exists at this point in the code. In this case we have just updated it in the previous 3 lines (89 -> 91).
0bdfdda
to
c824d56
Compare
@@ -1,4 +1,4 @@ | |||
import { DfuServiceBase, WsDfu } from "./wsdl/WsDfu/v1.65/WsDfu"; | |||
import { DfuServiceBase, WsDfu } from "./wsdl/WsDfu/v1.65/WsDfu.ts"; | |||
|
|||
export { WsDfu }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export type { WsDfu }?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of the others have export type
.
@@ -59,30 +54,30 @@ | |||
"wsdl-topology": "node ./lib-cjs/index.js -k --url=http://localhost:8010/WsTopology?wsdl --outDir=./src/services/wsdl", | |||
"wsdl-workunits": "node ./lib-cjs/index.js -k --url=http://localhost:8010/WsWorkunits?wsdl --outDir=./src/services/wsdl", | |||
"wsdl-all": "npm-run-all --aggregate-output -c --serial compile-util --parallel wsdl-access wsdl-account wsdl-cloud wsdl-codesign wsdl-dali wsdl-dfu wsdl-dfuxref wsdl-elk wsdl-esdlconfig wsdl-fileio wsdl-filespray wsdl-logaccess wsdl-machine wsdl-packageprocess wsdl-smc wsdl-resources wsdl-store wsdl-topology wsdl-workunits", | |||
"wsdl": "npm-run-all --aggregate-output -c --serial compile-util --parallel wsdl-access wsdl-account wsdl-cloud wsdl-codesign wsdl-dali wsdl-dfu wsdl-dfuxref wsdl-elk wsdl-fileio wsdl-filespray wsdl-logaccess wsdl-packageprocess wsdl-smc wsdl-store wsdl-topology wsdl-workunits" | |||
"wsdl": "npm-run-all --aggregate-output -c --serial compile-util --parallel wsdl-access wsdl-account wsdl-cloud wsdl-codesign wsdl-dali wsdl-dfu wsdl-dfuxref wsdl-elk wsdl-fileio wsdl-filespray wsdl-logaccess wsdl-packageprocess wsdl-smc wsdl-store wsdl-topology wsdl-workunits", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I noticed this yesterday and forgot to comment, but are "wsdl" and "wsdl-all" the same thing? Old merge conflict or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There not - going to keep the longer one (wsdl-all)
@@ -0,0 +1,4 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was this file added intentionally?
@@ -0,0 +1,39 @@ | |||
00000000 PRG 2024-09-25 11:32:05 7976 8104 "Error loading C:\Program Files\HPCCSystems\9.6.0\clienttools\filehooks\gitfile.lib: 193 - 193" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same for this one, i didn't see it getting used in any tests or anything
Changed type to "module" and dropped UMD support Signed-off-by: Gordon Smith <GordonJSmith@gmail.com>
c824d56
to
57d46af
Compare
Changed type to "module" and dropped UMD support
Checklist:
Testing: