-
Notifications
You must be signed in to change notification settings - Fork 386
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
feat: add LWS flag & import babel plugins #3357
feat: add LWS flag & import babel plugins #3357
Conversation
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
921899f
to
076fe6f
Compare
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
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.
LGTM! Awesome!
if (lws) { | ||
plugins.push( | ||
'@locker/babel-plugin-transform-unforgeables', | ||
'@babel/plugin-transform-async-to-generator', |
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.
☝️ For async-await support
plugins.push( | ||
'@locker/babel-plugin-transform-unforgeables', | ||
'@babel/plugin-transform-async-to-generator', | ||
'@babel/plugin-proposal-async-generator-functions' |
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.
☝️ For for await...of support
(despite the plugin name of "proposal" this feature is stage 4 and no longer a proposal)
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.
Can you add a sniff test to confirm that the lws
option works? Otherwise LGTM
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.
+1 for adding tests.
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
packages/@lwc/compiler/src/transformers/__tests__/transform-javascript.spec.ts
Outdated
Show resolved
Hide resolved
packages/@lwc/compiler/src/transformers/__tests__/transform-javascript.spec.ts
Outdated
Show resolved
Hide resolved
packages/@lwc/compiler/src/transformers/__tests__/transform-javascript.spec.ts
Outdated
Show resolved
Hide resolved
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 have some questions about this, will resolve internally
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
packages/@lwc/compiler/src/transformers/__tests__/transform-javascript.spec.ts
Outdated
Show resolved
Hide resolved
packages/@lwc/compiler/src/transformers/__tests__/transform-javascript.spec.ts
Show resolved
Hide resolved
packages/@lwc/compiler/src/transformers/__tests__/transform-javascript.spec.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
packages/@lwc/compiler/src/transformers/__tests__/transform-javascript.spec.ts
Outdated
Show resolved
Hide resolved
packages/@lwc/compiler/src/transformers/__tests__/transform-javascript.spec.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
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.
A deleted option has snuck in probably because of a rebase.
packages/@lwc/compiler/src/transformers/__tests__/transform-javascript.spec.ts
Show resolved
Hide resolved
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.
This LGTM after a few nitpicks, but I'd also like a +1 from @ravijayaramappa
Signed-off-by: Alex Chabot <a.chabot@salesforce.com>
8c87968
to
c7f7df7
Compare
Details
feat: add LWS flag & import babel plugins
Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?
GUS work item
@W-12560396 -- Enable LWS babel transform integration in OSS LWC compiler