add function for safely setting properties #2748
Conversation
🦋 Changeset detectedLatest commit: bc626e7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
| "setup": "npm run setup:ts-configs && npm run setup:patch && npm run setup:playwright && npm run setup:postinstall", | ||
| "setup:patch": "npx patch-package", | ||
| "setup:playwright": "npx playwright install", |
There was a problem hiding this comment.
For some reason Playwright changed their package to no longer install the browser instances with their package and now they need to be manually installed. This includes it as part of the setup. I can remove it, but nevs will need to run npx playwright install after each installation to ensure the browser instances are in sync the Playwright version.
| } else { | ||
| console.warn( | ||
| `Could not set property "${name}" on ${element.tagName} because it has no "setter".`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
We probably want to do something like https://github.com/thepassle/app-tools/tree/master/env so that console warnings can be trimmed from the final deployment in a production environment.
There was a problem hiding this comment.
Perfect! I was thinking the same thing, but wasn't sure if there was already a mechanism in place for this.
| } | ||
|
|
||
| function hasSetter(element: Element, name: string) { | ||
| return !!Object.getOwnPropertyDescriptor(element.constructor.prototype, name)?.set; |
| @@ -45,6 +50,7 @@ describe('spread', () => { | |||
| number, | |||
| '.array': array, | |||
| '.object': object, | |||
| '.validity': undefined, | |||
There was a problem hiding this comment.
What's the actual expectation associated with this?
There was a problem hiding this comment.
With this being set, it should not crash the application, but I can't get the tests to run for some reason. Are you able to for the lit helpers? They didn't seem to be set up at all.
|
Wtf is this
Jamie
…On Wed, Dec 27, 2023 at 7:07 AM Burton Smith ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In packages/lit-helpers/test-web/spread.test.js:
> @@ -45,6 +50,7 @@ describe('spread', () => {
number,
'.array': array,
'.object': object,
+ '.validity': undefined,
With this being set, it should not crash the application, but I can't get
the tests to run for some reason. Are you able to for the lit helpers? They
didn't seem to be set up at all.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
Was this comment meant for something else? |
What I did