feat: add API versioning #3350
feat: add API versioning #3350
Conversation
| @@ -37,8 +39,11 @@ export interface RollupLwcOptions { | |||
| enableScopedSlots?: boolean; | |||
| /** The configuration to pass to `@lwc/compiler` to disable synthetic shadow support */ | |||
| disableSyntheticShadowSupport?: boolean; | |||
| apiVersion?: APIVersion; | |||
There was a problem hiding this comment.
I'm assuming the apiVersion passed in to the @lwc/rollup-plugin will probably come from lwc.config.json or something (someday). It should be treated as a default that can be overridden by the js-meta.xml file.
| @@ -7,11 +7,13 @@ | |||
| import fs from 'fs'; | |||
| import path from 'path'; | |||
| import { URLSearchParams } from 'url'; | |||
| import { XMLParser } from 'fast-xml-parser'; | |||
There was a problem hiding this comment.
I don't have a strong opinion about XML parsers; I kind of just grabbed one at random.
I figure it's better to have something battle-tested than something fast – there could be a lot of XML files out there, and we want to be able to handle all of them.
| case APIFeature.DUMMY_FEATURE: | ||
| return apiVersion >= APIVersion.V59_246_WINTER_24; | ||
| } | ||
| } |
There was a problem hiding this comment.
This is a convenience function; it's used in the other PRs.
nolanlawson
force-pushed
the
nolan/api-version-1
branch
from
45734a0
to
45e6fc7
Compare
| @@ -16,6 +23,7 @@ import { VNodes } from './vnodes'; | |||
| import { checkVersionMismatch } from './check-version-mismatch'; | |||
|
|
|||
| const signedTemplateMap: Map<LightningElementConstructor, Template> = new Map(); | |||
| const templateToApiVersionMap: Map<LightningElementConstructor, APIVersion> = new Map(); | |||
There was a problem hiding this comment.
It seems like we won't be able to trust type checking when it comes to things like LightningElementConstructor, since that interface may change over time. Is there a strategy to ensure type-checked compatibility across internal interfaces between LOWEST_API_VERSION and latest?
There was a problem hiding this comment.
I'm not following you. LightningElementConstructor is not likely to change anytime soon.
That said, we could also just set it to any or function or something, since I find I end up have to do an as cast anyway.
There was a problem hiding this comment.
It may be that any unconcern I had is unwarranted. As you point out, LightningElementConstructor is not likely to change. However, for posterity:
I was pointing out that, at line 26, the type of Map<LightningElementConstructor, APIVersion> is referenced. However, that's actually not a type we can trust, because the run-time semantics of that type will differ from the compile-time semantics.
Because the implementation of LightningElementConstructor can vary across versions of LWC, the LightningElementConstructor that is referenced here should not be thought of as an interface, but rather as a union of all the LightningElementConstructors that might show up in the map with a corresponding API version. At runtime, that is. At compile time LightningElementConstructor will always refer to the interface that is implemented at the current commit. And type checking is done at run-time, leading to the disparity between run-time and compile-time semantics.
My question basically boiled down to: will there be a meta-LightningElementConstructor type that is unchanged across API versions, such that references to LightningElementConstructor in API-version-sensitive code will be provably correct? If we do not believe that LightningElementConstructor will be one of the things receiving breaking changes, then my question is probably moot.
There was a problem hiding this comment.
Because the implementation of
LightningElementConstructorcan vary across versions of LWC
I think I might misunderstand this part. Do you mean engine versions or API versions? Because there will only be one engine version on the client side (unless something is going very wrong). As for API version, that shouldn't matter because it won't change the identify of LightningElementConstructor. Unless I'm just totally misunderstanding.
|
|
||
| // These must be updated when the enum is updated. | ||
| // It's a bit annoying to do have to do this manually, but this makes the file tree-shakeable, | ||
| // passing the `verify-treeshakeable.js` test. |
There was a problem hiding this comment.
This doesn't seem to bad to me. Just add it to the list of tasks just prior to feature freeze.
There was a problem hiding this comment.
I considered an alternative, which is to use the /*#__PURE__*/ annotation. The problem with that is that the bundle size becomes larger. But it may be worth it if we can avoid a hard-to-remember manual task.
Also, we don't actually have to do this every feature freeze – only when we add a new APIFeature. If we skip a release, and there are no API changes in that release, then we can keep using the older one as the "latest".
Co-authored-by: Dale Bustad <dale@divmain.com>
nolanlawson
added
feature/api-versioning
Details
Fixes #3448
Implements API versioning as defined here. This is a 4-part PR.
This PR should not be merged yet. (Hence
nomerge.) The target is to ship this in 246. So 244 is treated as the "min" version.Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?
GUS work item
W-12985998