-
Notifications
You must be signed in to change notification settings - Fork 214
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
Enable useDefineForClassFields
TS flag
#7563
base: master
Are you sure you want to change the base?
Changes from all commits
9dc48ac
24628d7
60cf4f0
2842fe2
4867777
d0816f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"changes": [ | ||
{ | ||
"packageName": "@itwin/build-tools", | ||
"comment": "Enabled `useDefineForClassFields` TypeScript config flag", | ||
"type": "none" | ||
} | ||
], | ||
"packageName": "@itwin/build-tools" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"changes": [ | ||
{ | ||
"packageName": "@itwin/core-backend", | ||
"comment": "Changed `Model.parentModel`, `LightLocation.enabled` and some of the `AuxCoordSystem` properties to be optional", | ||
"type": "none" | ||
} | ||
], | ||
"packageName": "@itwin/core-backend" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"changes": [ | ||
{ | ||
"packageName": "@itwin/core-bentley", | ||
"comment": "", | ||
"type": "none" | ||
} | ||
], | ||
"packageName": "@itwin/core-bentley" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"changes": [ | ||
{ | ||
"packageName": "@itwin/core-common", | ||
"comment": "", | ||
"type": "none" | ||
} | ||
], | ||
"packageName": "@itwin/core-common" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"changes": [ | ||
{ | ||
"packageName": "@itwin/ecschema-metadata", | ||
"comment": "", | ||
"type": "none" | ||
} | ||
], | ||
"packageName": "@itwin/ecschema-metadata" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"changes": [ | ||
{ | ||
"packageName": "@itwin/presentation-common", | ||
"comment": "", | ||
"type": "none" | ||
} | ||
], | ||
"packageName": "@itwin/presentation-common" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -614,9 +614,13 @@ export class TemplateViewDefinition3d extends ViewDefinition3d { | |
*/ | ||
export abstract class AuxCoordSystem extends DefinitionElement { | ||
public static override get className(): string { return "AuxCoordSystem"; } | ||
public type!: number; | ||
public type?: number; | ||
public description?: string; | ||
public constructor(props: AuxCoordSystemProps, iModel: IModelDb) { super(props, iModel); } | ||
public constructor(props: AuxCoordSystemProps, iModel: IModelDb) { | ||
super(props, iModel); | ||
this.type = props.type; | ||
this.description = props.description; | ||
Comment on lines
+621
to
+622
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For fields like these (where the type annotation already matches what the |
||
} | ||
} | ||
|
||
/** A 2d auxiliary coordinate system. | ||
|
@@ -625,8 +629,12 @@ export abstract class AuxCoordSystem extends DefinitionElement { | |
export class AuxCoordSystem2d extends AuxCoordSystem { | ||
public static override get className(): string { return "AuxCoordSystem2d"; } | ||
public origin?: Point2d; | ||
public angle!: number; | ||
public constructor(props: AuxCoordSystem2dProps, iModel: IModelDb) { super(props, iModel); } | ||
public angle?: number; | ||
public constructor(props: AuxCoordSystem2dProps, iModel: IModelDb) { | ||
super(props, iModel); | ||
this.origin = props.origin ? Point2d.fromJSON(props.origin) : undefined; | ||
this.angle = props.angle ? Angle.fromJSON(props.angle).degrees : undefined; | ||
} | ||
|
||
/** Create a Code for a AuxCoordSystem2d element given a name that is meant to be unique within the scope of the specified DefinitionModel. | ||
* @param iModel The IModelDb | ||
|
@@ -645,10 +653,16 @@ export class AuxCoordSystem2d extends AuxCoordSystem { | |
export class AuxCoordSystem3d extends AuxCoordSystem { | ||
public static override get className(): string { return "AuxCoordSystem3d"; } | ||
public origin?: Point3d; | ||
public yaw!: number; | ||
public pitch!: number; | ||
public roll!: number; | ||
public constructor(props: AuxCoordSystem3dProps, iModel: IModelDb) { super(props, iModel); } | ||
public yaw?: number; | ||
public pitch?: number; | ||
public roll?: number; | ||
public constructor(props: AuxCoordSystem3dProps, iModel: IModelDb) { | ||
super(props, iModel); | ||
this.origin = props.origin ? Point3d.fromJSON(props.origin) : undefined; | ||
this.yaw = props.yaw ? Angle.fromJSON(props.yaw).degrees : undefined; | ||
this.pitch = props.pitch ? Angle.fromJSON(props.pitch).degrees : undefined; | ||
this.roll = props.roll ? Angle.fromJSON(props.roll).degrees : undefined; | ||
Comment on lines
+661
to
+664
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are nice fixes. Frankly I'm surprised that no one has hit this in all these years... Can you also add unit tests that exercise this so we can avoid regressions as we refactor all these |
||
} | ||
|
||
/** Create a Code for a AuxCoordSystem3d element given a name that is meant to be unique within the scope of the specified DefinitionModel. | ||
* @param iModel The IModelDb | ||
|
@@ -700,7 +714,10 @@ export class ViewAttachment extends GraphicalElement2d { | |
export class LightLocation extends SpatialLocationElement { | ||
public static override get className(): string { return "LightLocation"; } | ||
/** Whether this light is currently turned on. */ | ||
public enabled!: boolean; | ||
public enabled?: boolean; | ||
|
||
protected constructor(props: LightLocationProps, iModel: IModelDb) { super(props, iModel); } | ||
protected constructor(props: LightLocationProps, iModel: IModelDb) { | ||
super(props, iModel); | ||
this.enabled = props.enabled; | ||
} | ||
} |
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.
This class field is interesting... @GytisCepk were there any test or build failures that prompted you to add this?
I did some debugging and
elementGeometryBuilderParams
definitely is not set by theEntity
constructor, and it also doesn't get serialized inGeometricElement.toJSON()
. However, the correspondingGeometricElementProps.elementGeometryBuilderParams
member is used in a bunch of places...@pmconne @bbastings is it possible this class field is just broken and has never been used? Should we remove it entirely? If we want to fix it instead, we should probably also make sure it gets included in
toJSON()
...