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

fix: syntax errors after ember 4 upgrade #899

Merged
merged 2 commits into from
Mar 10, 2023
Merged

fix: syntax errors after ember 4 upgrade #899

merged 2 commits into from
Mar 10, 2023

Conversation

adong
Copy link
Contributor

@adong adong commented Mar 10, 2023

Context

Sometimes we see broken pages for these 2 reasons:

  1. Strict mode in handlebars template files template.hbs.

Error message sample:

Uncaught (in promise) Error: Attempted to resolve a value in a strict mode template, but that value was not in scope: isShowingModal

image

Before:

{{#if isShowingModal}}

After

{{#if this.isShowingModal}}

See https://github.com/emberjs/rfcs/blob/master/text/0496-handlebars-strict-mode.md#2-no-implicit-this-fallback

  1. Change the implicit set to explicit this.set, for Ember version 3.20.0+ for an implicit set.

2023-03-10_11-49-59 (1)

Error Message:

Uncaught (in promise) Error: Assertion Failed: You attempted to update <screwdriver-ui@model:job::ember545:4816773>._isDisabled to "false", but it is being tracked by a tracking context, such as a template, computed property, or observer. In order to make sure the context updates properly, you must invalidate the property when updating it. You can mark the property as `@tracked`, or use `@ember/object#set` to do this.

Before:

routeParams: computed('model', {
    set(_, value) {
      return (this._routeParams = value);
    }
}

After:

routeParams: computed('model', {
    set(_, value) {
      this.set('_routeParams', value);

      return value;
    }
}

You must return the new intended value of the computed property from the setter function.

I also updated to have return value explicitly as well, from

return this.set('_isInactivePipeline', value);

To

this.set('_isInactivePipeline', value);

return value;

Objective

This PR is merely to solve the blank pages issue, and we need to consider a path to move forward and use the latest glimmer syntax.

Such as: we need to move away from computed properties
image

To tracked computed values
image

References

  1. computed properties
  2. Why do I not need to mark the property as tracked?

screwdriver-cd/screwdriver#2838

License

I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.

adong added 2 commits March 10, 2023 10:53

Verified

This commit was signed with the committer’s verified signature. The key has expired.
juliusknorr Julius Knorr
Copy link
Member

@jithine jithine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

@adong adong merged commit 4e095ff into master Mar 10, 2023
@adong adong deleted the adong/syntax-errors branch March 10, 2023 20:15
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.

None yet

2 participants