Update plugin to new provider service #141

jerone · 2016-05-15T21:53:51Z

I think it's nice to let plugin developers know that there's a new provider service available so we can remove the legacy code as soon as possible. Besides that, the previously documented provider semver won't support the new version we're releasing.

There are two lists of plugins:

Plugins that consume the service and the public API.
Plugins that consume the service and use the return type.

List one:

List two:

Flex Tool Bar ➡️ cakecatz/flex-toolbar@6254e0e
Nuclide ➡️ facebookarchive/nuclide@173f26e
Atom-macros (in an invalid way btw) ➡️ Update tool-bar package provider service & API pbihler/atom-macros#8

~~This is a work in progress as there are way more plugins available.~~ List seems complete for the moment.

Ref #135, #136, #140.

The text was updated successfully, but these errors were encountered:

zertosh · 2016-05-16T07:03:12Z

Nuclide does tuesday releases (internally and open-source), so if you publish the new tool-bar on Monday morning, then that gives me a day to update. This would be the ideal case.

jerone · 2016-05-16T20:11:13Z

I'm going to hold of publishing for a little bit. This issue was opened to get a list of plugins using the tool-bar API and informing them of the changes before releasing the package. The list was summarized out of the top of my head, but requires some more research. The idea right now is to update the list, at least informing the package authors and maybe sending an PR.

…ol-bar#141][].

jerone · 2016-05-18T18:33:28Z

Can someone confirm that using ^0 || ^1 is the correct semver range for using in the new service provider...

/cc @suda @cakecatz @zertosh

zertosh · 2016-05-18T18:41:37Z

Yeah...

require('semver').toComparators('^0 || ^1')
// [ [ '>=0.0.0', '<1.0.0' ], [ '>=1.0.0', '<2.0.0' ] ]

jerone · 2016-05-18T18:54:59Z

Thanks for the confirmation on the ranges. Some explanation. Previously we had ^0.1.0 but I learned that that would only select >=0.1.0 <0.2.0. For 1.0.0 I want to make sure we get all new minor updates too. Seems valid I think.

zertosh · 2016-05-18T19:02:58Z

Yup. See https://github.com/npm/node-semver#caret-ranges-123-025-004

[Caret Ranges allow] changes that do not modify the left-most non-zero digit in the [major, minor, patch] tuple. In other words, this allows patch and minor updates for versions 1.0.0 and above, patch updates for versions 0.X >=0.1.0, and no updates for versions 0.0.X.

jerone · 2016-05-18T19:55:40Z

Because of the very hard work and if you agree, I like you to add to the list of contributors on the readme.

In #140 (comment) you mentioned that there is room for improvements and performance. I'm especially interested in your thoughts about performance improvements.

zertosh · 2016-05-19T16:38:41Z

Thanks @jerone! 😄

zertosh · 2016-05-19T16:41:20Z

@jerone as far as perf:

Dynamically load the stylesheets for the icon sets you use. This will cut the startup time in half.
Debounce with requestAnimationFrame the resize and scroll events.
Insert the initial buttons in a single batch. Right now you're forcing a relayout for every button at startup. Really expensive.

jerone · 2016-05-23T09:37:23Z

This is an important one! The iconsets are "huge", so if we can somehow lazy load those files the startup time would indeed decrease. Besides that point, I can also see conflicts with other packages, when they load those iconsets too.
Both scroll on the tool-bar and resize on the window won't happen that much, but still it's an good idea to debounce those events.
Aldo I see the benefit of this (forcing redraw events), I don't see how we can improve this. At anytime it's possible to addItem to the tool-bar, not only at startup (or when the service is called).

zertosh · 2016-05-23T16:18:27Z

Both scroll on the tool-bar and resize on the window won't happen that much, but still it's an good idea to debounce those events

I resize my window a lot - probably because I'm always opening/closing the devtools. This is easy to fix though.

Aldo I see the benefit of this (forcing redraw events), I don't see how we can improve this. At anytime it's possible to addItem to the tool-bar, not only at startup (or when the service is called).

I'm only concerted with batching the DOM updates during the Atom package activation cycle. We put 13 buttons on the tool-bar - that's a lot of redrawing. I'm on a mission to reduce start up time :)

zertosh · 2016-06-08T21:47:47Z

Any word on publishing the space-view-less update?

jerone · 2016-06-09T19:20:33Z

@zertosh commented on 8 jun. 2016 23:47 CEST:

Any word on publishing the space-view-less update?

Before publishing the package with the awesome new changes we need to contact all plugins (send PR) so they have time to react on the breaking changes. I've already started to research some plugins above, but the list is not yet finished.
Currently I'm to busy to finish the list and send PR's. But in 1,5 week I have some vacation time to spend on this issue.

I would be great if others could help. /cc @suda @cakecatz

📦 ⬆️ Update tool-bar package provider service. Per atom-community/tool-bar#141

jerone · 2016-07-01T18:47:12Z

Half of above mentioned packages are ready for a new Tool Bar release, with remaining package on very low install counts.

I plan on releasing a new version this Sunday.

@zertosh @cakecatz Any objections?

zertosh · 2016-07-03T04:12:04Z

@jerone We're good on my side! Thanks!

jerone · 2016-07-03T14:48:26Z

cakecatz · 2016-07-04T03:05:25Z

Do you have time to update Flex Tool Bar with the latest API changes ... so it's compatible when we release then new Tool Bar package.

@jerone Sorry, I had no time to do that... 😫😫😫
But I will working for it from today 💪

Summary: The `tool-bar` package will soon release v1.0.0. In this update, it has dropped `space-pen`, so `addButton` no longer returns a `space-pen` view. However, it does expose the underlaying DOM element the same way - via an `element` property. This diff changes our `tool-bar` usage to always use `element` (when needed), and sets the service API version requirement to `^0 || ^1` (since this is a backwards-compat change). A lot of packages weren't properly disposing of `tool-bar` related resources. This was most obvious with packages that showed state on `tool-bar` element. If `tool-bar` was disabled, and then re-enabled, the counters would never update. Less obvious was that by subscribing to the cleanup disposable when either the tool-bar or the package was disabled, meant that the other side's disposable still held a reference to the toolbar. See: atom-community/tool-bar#141. Reviewed By: jamesgpearce Differential Revision: D3513888 fbshipit-source-id: f995d3e9c686876d36924054cf1dd2f88e1650a7

jerone · 2016-07-13T11:31:17Z

Closing, as all plugins have been notified and/or updated.

jerone · 2016-09-06T20:50:59Z

@cakecatz Almost 5 months after removing space-pen dependency and switching to ES6, it's time to merge #153 and remove all legacy code. The only thing blocking this is the Flex Toolbar package still running on the old API (most noticeable packages have already switched). Once merged, Flex Toolbar will completely stop working. I noticed you already made an attempt to upgrade on a different branch. Is there any change to speed up this process?

/cc @zertosh

zertosh · 2016-09-07T02:42:08Z

@jerone, we're all good on my end

cakecatz · 2016-09-07T09:35:35Z

@jerone Hi! I pushed commit for support new tool-bar provider just now.

jerone · 2016-09-07T10:03:49Z

@cakecatz Great! Do you want me to test? I probably have some spare time tonight.

cakecatz · 2016-09-07T10:15:43Z

@jerone Yes please 🙏

jerone · 2016-09-07T18:06:21Z

@cakecatz commented on 7 sep. 2016 12:15 CEST:

@jerone Yes please 🙏

Just tested your latest commit with #153 and it worked with my configuration. Let me know when you released Flex Tool Bar, so I can merge #153.

cakecatz · 2016-09-07T18:25:30Z

@jerone Thanks!
I'll release now 🚢

…munity/tool-bar#141

thiagolucio · 2016-10-17T15:50:46Z

Hi Jerone.
So..i correct the package last Saturday Night. Thanks to warning. I think ok now. The package is:
"toolbar-iconshortcuts"
Thanks Again.

jerone added WIP needs more information labels May 15, 2016

jerone added a commit to jerone/menu-manager that referenced this issue May 16, 2016

📦 ⬆️ Update tool-bar package provider service. Per [atom-community/to…

682e7f4

…ol-bar#141][].

jerone mentioned this issue May 16, 2016

Update tool-bar package provider service jerone/menu-manager#4

Merged

jerone added a commit to jerone/menu-manager that referenced this issue May 16, 2016

📦 ⬆️ Update tool-bar package provider service. Per [atom-community/to…

b303cec

…ol-bar#141][].

jerone added help wanted team labels May 16, 2016

jerone mentioned this issue May 17, 2016

Update tool-bar package provider service suda/tool-bar-main#15

Merged

jerone added a commit to suda/tool-bar-main that referenced this issue Jun 26, 2016

Merge pull request #15 from jerone/issues/tool-bar-dep-upgrade

42aa248

📦 ⬆️ Update tool-bar package provider service. Per atom-community/tool-bar#141

This was referenced Jun 26, 2016

📦 ⬆️ Update tool-bar package provider service varemenos/atom-toolbar-almighty#15

Merged

📦 ⬆️ Update tool-bar package provider service particle-iot-archived/particle-dev#150

Merged

jerone added a commit to jerone/menu-manager that referenced this issue Jun 26, 2016

Merge pull request #4 from jerone/issues/tool-bar-dep-upgrade

2978187

📦 ⬆️ Update tool-bar package provider service. Per atom-community/tool-bar#141

jerone self-assigned this Jul 1, 2016

jerone removed help wanted needs more information labels Jul 1, 2016

jerone closed this as completed Jul 13, 2016

jerone removed the WIP label Jul 13, 2016

jerone changed the title ~~[WIP] Update plugin to new provider service~~ Update plugin to new provider service Jul 13, 2016

jerone mentioned this issue Jul 14, 2016

🔥 Remove legacy spece-pen view api compat #153

Merged

jerone removed their assignment Sep 7, 2016

thiagolucio added a commit to thiagolucio/toolbar-iconshortcuts that referenced this issue Oct 15, 2016

Normalize Package from new compliance of toolbar of suda Ref atom-com…

a72a501

…munity/tool-bar#141

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update plugin to new provider service #141

Update plugin to new provider service #141

jerone commented May 15, 2016 •

edited

Loading

zertosh commented May 16, 2016

jerone commented May 16, 2016

jerone commented May 18, 2016

zertosh commented May 18, 2016

jerone commented May 18, 2016

zertosh commented May 18, 2016

jerone commented May 18, 2016

zertosh commented May 19, 2016

zertosh commented May 19, 2016

jerone commented May 23, 2016

zertosh commented May 23, 2016

zertosh commented Jun 8, 2016

jerone commented Jun 9, 2016

jerone commented Jul 1, 2016

zertosh commented Jul 3, 2016

jerone commented Jul 3, 2016

cakecatz commented Jul 4, 2016

jerone commented Jul 13, 2016

jerone commented Sep 6, 2016

zertosh commented Sep 7, 2016

cakecatz commented Sep 7, 2016

jerone commented Sep 7, 2016

cakecatz commented Sep 7, 2016

jerone commented Sep 7, 2016

cakecatz commented Sep 7, 2016

thiagolucio commented Oct 17, 2016

Update plugin to new provider service #141

Update plugin to new provider service #141

Comments

jerone commented May 15, 2016 • edited Loading

zertosh commented May 16, 2016

jerone commented May 16, 2016

jerone commented May 18, 2016

zertosh commented May 18, 2016

jerone commented May 18, 2016

zertosh commented May 18, 2016

jerone commented May 18, 2016

zertosh commented May 19, 2016

zertosh commented May 19, 2016

jerone commented May 23, 2016

zertosh commented May 23, 2016

zertosh commented Jun 8, 2016

jerone commented Jun 9, 2016

jerone commented Jul 1, 2016

zertosh commented Jul 3, 2016

jerone commented Jul 3, 2016

cakecatz commented Jul 4, 2016

jerone commented Jul 13, 2016

jerone commented Sep 6, 2016

zertosh commented Sep 7, 2016

cakecatz commented Sep 7, 2016

jerone commented Sep 7, 2016

cakecatz commented Sep 7, 2016

jerone commented Sep 7, 2016

cakecatz commented Sep 7, 2016

thiagolucio commented Oct 17, 2016

jerone commented May 15, 2016 •

edited

Loading