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

Pre-processing translation strings stop working in v1.9.106/107 #6967

Closed
SamMousa opened this issue Sep 15, 2023 · 4 comments
Closed

Pre-processing translation strings stop working in v1.9.106/107 #6967

SamMousa opened this issue Sep 15, 2023 · 4 comments
Assignees
Labels
bug user issue An issue or bug reported by users
Milestone

Comments

@SamMousa
Copy link
Contributor

SamMousa commented Sep 15, 2023

Are you requesting a feature, reporting a bug or asking a question?

Bug

What is the current behavior?

We discovered this with custom properties, but reproduced it with built in properties.
Doing something like this: Survey.surveyLocalization.locales.en.startSurveyText = "{n}";
Will break the survey model constructor.

What is the expected behavior?

This is a regression.

How would you reproduce the current behavior (if this is a bug)?

Survey.surveyLocalization.locales.en.startSurveyText = "{n}";
new Survey.Model();

Provide the test code and the tested page URL (if applicable)

Check the developer console to see the error!

Tested page URL: https://plnkr.co/edit/7srGU5BPMpf66E90

Specify your

  • surveyjs platform (angular or react or jquery or knockout or vue): knockout
  • surveyjs version: 107

Important

While the given example can be fixed by moving up the initialization of this.textPreProcessor in the Survey constructor, this will not fix the issue for custom properties.

Custom properties are added in the super() call by a parent class.
The issue is caused by a circular reference in base.ts:

public createCustomLocalizableObj(name: string) {
    var locStr = this.getLocalizableString(name);
    if (locStr) return;
    this.createLocalizableString(name, <ILocalizableOwner>(<any>this), false, true);
  }

This localizable string is passed the survey class as an implementor of ILocalizableOwner. But at this moment it cannot yet fully satisfy this interface since it has not been fully initialized...

The design flaw becomes obvious when you look at the typecasting needed to silence typescript.
I'm not sure what the best solution to untangle this is...

One possible solution that might work, without fixing the design, is to simply initialize the field in the declaration, not in the constructor:

private textPreProcessor: TextPreProcessor = new TextPreProcessor();
@andrewtelnov andrewtelnov self-assigned this Sep 15, 2023
@andrewtelnov andrewtelnov added bug user issue An issue or bug reported by users labels Sep 15, 2023
@andrewtelnov andrewtelnov changed the title Localizable properties broken in >= 106 Pre-processing translation strings stop working in v1.9.106/107 Sep 16, 2023
@andrewtelnov
Copy link
Member

@SamMousa It is fixed by the commit above. I created the instance on the first request.

Thank you,
Andrew

@SamMousa
Copy link
Contributor Author

That'll work as well! Why did you prefer it above initialisation at the point of declaration? You use that method for other properties

@andrewtelnov
Copy link
Member

@SamMousa I need to setup this class instance. It is not a constructor call only.

Thank you,
Andrew

@OlgaLarina OlgaLarina added this to the v1.9.108 milestone Sep 22, 2023
@SamMousa
Copy link
Contributor Author

SamMousa commented Oct 23, 2023

Please reopen this, in v1.9.113 it's back in a different form.

  public getVariable(name: string): any {
    if (!name) return null;
    name = name.toLowerCase();
    var res = this.variablesHash[name];
    if (!this.isValueEmpty(res)) return res;
    if (name.indexOf(".") > -1 || name.indexOf("[") > -1) {
      if (new ProcessValue().hasValue(name, this.variablesHash))
        return new ProcessValue().getValue(name, this.variablesHash);
    }
    return res;
  }

Specifically this.variablesHash is access before being initialized.
image

It's trying to resolve the variable but failing to do so because the survey object is not fully initialized.
I think it's easier / more logical to handle translations after all initialization has been done.

Edit: I think this may never have actually been fixed, there were just more errors below the first one. In 106 it's broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug user issue An issue or bug reported by users
Projects
None yet
Development

No branches or pull requests

3 participants