Skip to content

Rerender with template throws "Cannot redefine property" #222

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

Closed
rothsandro opened this issue Jun 2, 2021 · 3 comments · Fixed by #223
Closed

Rerender with template throws "Cannot redefine property" #222

rothsandro opened this issue Jun 2, 2021 · 3 comments · Fixed by #223

Comments

@rothsandro
Copy link
Contributor

Steps to reproduce

  1. Create a new Angular 12 project
  2. Install testing library
  3. Add a new test that renders a template with componentProperties
  4. Call "rerender" with an updated property value
const { rerender, getByText } = await render(`<div>Hello {{ name}}</div>`, {
  componentProperties: {
    name: 'Sarah',
  },
});

getByText('Hello Sarah');
rerender({ name: 'Mark' });  // Throws the error
getByText('Hello Mark');});

Expected behavior

The template should be updated with the new value

Actual behavior

The following error is thrown:

TypeError: Cannot redefine property: name

Cause

The function setComponentProperties which is called during initial render and during rerender uses Object.defineProperty. As configurable is false by default (see MDN) this cannot be called twice and throws the error above on rerender.

The function should set configurable: true.

Demo

https://github.com/rothsandro/ng-tlib-template-rerender/blob/main/src/app/rerender-template.spec.ts

@timdeschryver
Copy link
Member

@rothsandro thanks for the detailed info!
If you want, feel free to create a Pull Request with the needed changes.
The proposition seems fine to me.

@rothsandro
Copy link
Contributor Author

@timdeschryver Thanks. Yes, I can create a PR.

I already started creating a test case for that before fixing it but it seems that Jest has configurable true by default (vs. false with Karma and a real browser) so the test does not fail :-( Should I just fix this without a new test case?

@timdeschryver
Copy link
Member

@rothsandro oh, good to know.
If you add a comment why we do it, then that's fine.
Perhaps that we should create some karma tests for safety in the future...

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 a pull request may close this issue.

2 participants