-
Notifications
You must be signed in to change notification settings - Fork 15
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
chore(package): update lwc- and eslint-related deps #42
Conversation
"@lwc/compiler": "^2.1.0", | ||
"@lwc/engine-dom": "^2.1.0", | ||
"@lwc/eslint-plugin-lwc": "^1.0.1", | ||
"@lwc/jest-preset": "^11.0.1", |
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 don't think we need to import each package discretely. @lwc/compiler
, @lwc/engine-dom
, @lwc/synthetic-shadow
should be able to be replaced by lwc
.
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.
@pmdartus It was already working that way in the existing code, so I didn't change it.
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 am fine either way, it something that can be fixed later if needed.
@@ -26,18 +26,22 @@ | |||
], | |||
"repository": "https://github.com/salesforce/wire-service-jest-util", | |||
"peerDependencies": { | |||
"@lwc/engine": "^1.5.0" | |||
"@lwc/engine-dom": "^2.0.0" |
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.
What about using a peer dependency against the umbrella package instead of @lwc/engine-dom
? IMO, we should avoid asking developers to import each piece of the LWC framework independently.
"@lwc/engine-dom": "^2.0.0" | |
"lwc": "^2.0.0" |
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 brought up this concern for salesforce/lwc-test#110 (comment) but we ended up not doing so after some discussion. @jodarove do you remember what the reasoning was?
BREAKING CHANGE: This will bump the peer dependency from
@lwc/engine
v1 to@lwc/engine-dom
v2.