fix: disable form association entirely on older API versions #4041
Conversation
nolanlawson
force-pushed
the
nolan/form-assoc
branch
from
773bded to
7e15c10
Compare
nolanlawson
changed the title
| `The attachInternals API is only supported in API version 61 and above. To use this API please update the LWC component API version.` | ||
| `The attachInternals API is only supported in API version 61 and above. ` + | ||
| `The current version is ${apiVersion}. ` + | ||
| `To use this API, update the LWC component API version. https://lwc.dev/guide/versioning` |
There was a problem hiding this comment.
Guidelines for user messages say "don't say 'please'".
| @@ -38,12 +38,12 @@ function createPreprocessor(config, emitter, logger) { | |||
|
|
|||
| // Wrap all the tests into a describe block with the file structure name | |||
| // This avoids needing to manually write `describe()` for every file. | |||
| // Also add a dummy test because otherwise Jasmine complains about empty describe()s: | |||
| // Also add an empty test because otherwise Jasmine complains about empty describe()s: | |||
There was a problem hiding this comment.
I forgot we were trying to remove this term since it can be misconstrued.
nolanlawson
changed the title
| elm = createElement(tagName, { is: ctor }); | ||
| }; | ||
| if (ENABLE_ELEMENT_INTERNALS_AND_FACE) { | ||
| doCreate(); |
There was a problem hiding this comment.
Could we use process.env.SYNTHETIC_SHADOW_ENABLED here instead of !process.env.NATIVE_SHADOW?
Unless it's possible for process.env.FORCE_NATIVE_SHADOW_MODE_FOR_TEST and process.env.SYNTHETIC_SHADOW_ENABLED to be enabled at the same time.
There was a problem hiding this comment.
I kind of consider process.env.NATIVE_SHADOW to be the "default" choice for tests, and that SYNTHETIC_SHADOW_ENABLED is only needed for odd cases where you need to do something differently in FORCE_NATIVE_SHADOW_MODE_FOR_TEST mode.
Admittedly FORCE_NATIVE_SHADOW_MODE_FOR_TEST is kind of weird, so it's confusing. Maybe we should have better names here.
packages/@lwc/integration-karma/test/component/face-callbacks/index.spec.js
Outdated
Show resolved
Hide resolved
| toThrowCallbackReactionErrorEvenInSyntheticLifecycleMode: errorMatcherFactory( | ||
| windowErrorListener, | ||
| true | ||
| ), |
There was a problem hiding this comment.
@jmsjtu This is kind of weird but I couldn't think of a better name for this 😂
There was a problem hiding this comment.
😆 how about toThrowCallbackReactionErrorInAllDomModes, what you have works for me too.
We can always change it later if we come up with a better name!
* refactor: organize API versioning tests (#4040) * fix: disable form association entirely on older API versions (#4041) Fixes #3929 * Revert "fix(synthetic-shadow): retargeting does not correctly account for nat…" (#4047) This reverts commit 13cd45f. * chore: routine dependencies update (#4046) * chore: routine dependencies update * fix: revert prettier updates * chore: update husky * chore: update husky * fix: disable ts rule * chore: v6.3.2 (#4048) --------- Co-authored-by: Nolan Lawson <[email protected]> Co-authored-by: Eugene Kashida <[email protected]>
Details
Fixes #3929
Logs a warning when using synthetic custom element lifecycle and attempting to use
static formAssociated = true. Also, ensuresformAssociatedis always false when the API version is <61.Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?
Technically someone could have been trying to use an older API version and setting formAssociated to true, but the FACE callbacks wouldn't have worked and older API versions are not used in OSS so I really doubt anyone took a dependency on this.