Skip to content
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

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

jmsjtu
Copy link
Member

@jmsjtu jmsjtu commented Jun 8, 2023

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 in core which is currently in progress (W-13519768).

Does this pull request introduce a breaking change?

  • 🚨 Yes, it does introduce a breaking change.

This change will break any users of lwc:dynamic.

Does this pull request introduce an observable change?

  • ⚠️ Yes, it does include an observable change.

The lwc:dynamic will no longer be a valid template directive.

GUS work item

W-12607866

@jmsjtu jmsjtu requested a review from a team as a code owner June 8, 2023 03:27
@@ -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.
Copy link
Collaborator

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a plan!

Copy link
Member Author

@jmsjtu jmsjtu Jun 9, 2023

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.

Copy link
Member Author

@jmsjtu jmsjtu Jun 16, 2023

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.

Copy link
Collaborator

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?

@jmsjtu jmsjtu requested review from nolanlawson and a team June 16, 2023 04:54
@nolanlawson
Copy link
Collaborator

/nucleus test

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 this pull request may close these issues.

2 participants