-
Notifications
You must be signed in to change notification settings - Fork 393
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
refactor: remove lwc-dynamic directive #3562
base: master
Are you sure you want to change the base?
Conversation
@@ -43,7 +43,6 @@ const { code } = transformSync(source, filename, options); | |||
- `namespace` (type: `string`, required) - namespace of the component, e.g. `x` in `x/foo`. | |||
- `stylesheetConfig` (type: `object`, default: `{}`) - The stylesheet compiler configuration to pass to the `@lwc/style-compiler`. | |||
- `experimentalDynamicComponent` (type: `DynamicImportConfig`, default: `null`) - The configuration to pass to `@lwc/compiler`. | |||
- `experimentalDynamicDirective` (type: `boolean`, default: `false`) - The configuration to pass to `@lwc/template-compiler` to enable deprecated dynamic components. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid breaking downstreams, can we just force it to be false
regardless of what's passed in? This is what we did for minify
and are going to do for lwc:spread #3544.
Then we can add it to the breaking changes wishlist to actually remove the config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like a plan!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nolanlawson now that I think about it, the experimentalDynamicDirective
option was released at the start of 246 and I think the only consumers are lwc-platform
.
I think the experimentalDynamicComponent
option may be the one that affects more downstream dependencies.
I'll double-check to see if anyone else is using experimentalDynamicDirective
but otherwise I think it should be safe to remove that option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked all of our internal repos and it looks like the only usage of this is in lwc-platform
, I think it should be safe to remove the experimentalDynamicDirective
option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jmsjtu Should this be green in Nucleus before merging?
/nucleus test |
Details
In preparation for the new dynamic components syntax to go GA, this PR removes all references to the
lwc:dynamic
directive (#3331).This PR cannot be merged until the usages of
lwc:dynamic
have been removed incore
which is currently in progress (W-13519768).Does this pull request introduce a breaking change?
This change will break any users of
lwc:dynamic
.Does this pull request introduce an observable change?
The
lwc:dynamic
will no longer be a valid template directive.GUS work item
W-12607866