-
Notifications
You must be signed in to change notification settings - Fork 69
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
Migrate link-item to typescript #7080
Conversation
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
Size Change: -14.6 kB (-1%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
@ismaeldcom I've updated the target in tsconfig to ES6, can you please check it once. |
Hey @naman03malhotra! I wonder what's the use case for the tsconfig target update. I see The target option in TS config stand for which target of JavaScript will be emitted, but doesn't affect which TS/JS features we can use while coding. Although, with our current browser support it's probably okay to start targeting ES6 instead of ES5 🙂 |
With the target as ES5, ts throws an error regarding the usage of ES6 goodies. There are other ways to solve it, but I found this the recommended way. |
This what you get when using spread operator with ES5 as target.
I think it should be safe to update the target to ES6 with the present browser support. |
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.
Oh! I thought it was due to the use of the spread operator, but it's due to the iteration of Set
. Yep, updating the target to ES6 is probably the easiest way to do it.
I just checked our current supported browsers, and can confirm that it's safe to use ES6 from now on 🙂
@ismaeldcom can you please take a look at #7081 which is an extension of this PR :) |
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! I tested the changes on my local env. and everything is working.
Fixes #7052
Changes proposed in this Pull Request
link-item.js
to typescript.es6
intsconfig
to allow spread operator.Testing instructions
npm run changelog
to add a changelog file, choosepatch
to leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge