Skip to content
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

Upgrade core-backend to ES2022 #6613

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Upgrade core-backend to ES2022 #6613

wants to merge 2 commits into from

Conversation

pmconne
Copy link
Member

@pmconne pmconne commented Apr 12, 2024

Don't merge - for demonstration purposes only.

This produces a slew of test failures, occurring when inserting a model, because modeledElement is undefined.

Model constructor calls Entity constructor. When Entity constructor exits, this.modeledElement is valid. When execution returns to Model constructor, this.modeledElement is undefined.

The property gets assigned via black magic in Entity.forEachProperty. Something in ES2022 changes the behavior there.

I had upgraded core-backend to target ES2022 in #6572 because TypeScript doesn't provide type declarations for Intl.Segmenter for older targets.

@pmconne pmconne requested a review from a team as a code owner April 12, 2024 13:38
@kabentley
Copy link
Contributor

kabentley commented Apr 12, 2024

maybe the constructor for Entity should be:

  protected constructor(props: EntityProps, iModel: IModelDb) {
    this.iModel = iModel;
    this.id = Id64.fromJSON(props.id);
    // copy all auto-handled properties from input to the object being constructed
    iModel.forEachMetaData(props.classFullName, true, (propName: string, meta: PropertyMetaData) => (this as any)[propName] = meta.createProperty((props as any)[propName]), false);
  }

I don't know if that will help but we won't be permuting this while we're calling a method on it.

@aruniverse aruniverse marked this pull request as draft May 17, 2024 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants