Fix importing for touch-bar-manager

[thatcomputerguy0101]

This fixes an error when importing the touch-bar-manager script where the module object was imported but the constructor function was not extracted.

However, there still exists a problem where provideToolBar can be called before useTouchBar returns, so something has to be done about that. One option to fix this is reverting the conditional loading of touch-bar-manager (the edits to tool-bar.js in d172cce). Another option would be to tie the provideToolBar function to the completion of the useTouchBar function, but I don't know how to do that without making provideToolBar async, which changes the API.

Fixes #325

[ericcornelissen]

@aminya, do you have any preference regarding the remaining problem?

[thatcomputerguy0101]

I looked into a solution for the second part of this problem, and apparently, the activate method is supposed to be async compatible. That came from the Atom flight manual, where it says:

activate(state): This optional method is called when your package is activated. It is passed the state data from the last time the window was serialized if your module implements the serialize() method. Use this to do initialization work when your package is started (like setting up DOM elements or binding events). If this method returns a promise the package will be considered loading until the promise resolves (or rejects).

However, the relationship between a package being considered "loaded" and its service provider functions being called doesn't appear to be well defined, as provideToolBar would be called even when returning the promise from calling useTouchBar or making the whole activate function async. My understanding of Atom's current source code is that it doesn't do anything with the return value of the activate function. Looking at Atom's issues, it seems as if this may be related to atom/atom#18798, which makes it seem like this capability is still being evaluated for existing compatibility before it is implemented.

The solution that I settled on as a temporary measure for myself was to remove the conditional loading for the touch-bar-manager script and add the file to the primary imports at the top, which is essentially the first option from the original post.

[aminya]

@aminya, do you have any preference regarding the remaining problem?

I'll take a look tonight. Sorry I missed this PR.

[idleberg]

This PR fixes the error I'm getting at startup (I don't have a touch bar), so please let me bump the issue

[GRA0007]

This fixes the issue for me too, I was also having an issue on line 18 of touch-bar-manager.js; it should be checking if icon isn't null in that function as sometimes it can be if there are some invalid items in the toolbar.

I am also still having an issue where the touchbar icons don't load unless I reload just the toolbar package I'm using because by the time the TouchBarManager has been loaded in (tool-bar.js:11), all the addButton calls have already been made and so they don't get added to the touch bar. The only way I can think of to fix this is to import the touchbar manager always, but that's not ideal for non-touchbar users. If y'all have any ideas of how to fix this, please let me know.

[aminya]

@suda

[thatcomputerguy0101]

It seems like the fix in this pull request has helped out a few people. If this pull request should be merged as is, I can take it out of draft mode so that it can be merged. Otherwise, I will leave this in draft mode until the problem with provideToolBar also has a fix. As for fixing that problem, I am still waiting on a preference for which approach I should attempt to use to fix the problem. Reverting the addition of conditional loading for the touch-bar-manager script will definitely work, but comes at a cost of additional loading time for users that don't have a touch bar. Keeping the conditional loading comes with three options: changing the outward facing API for provideToolBar so that it is async, getting Atom to implement asynchronous package loading (as proposed in atom/atom#18798), or using a package such as abbr/deasync to act as a async to sync wrapper. However, the first of those options breaks existing compatibility, the second requires changes to external code, and the deasync package for the third solution doesn't seem to be actively maintained anymore or have support for Promise based async sources.

[GRA0007]

It might be worth also considering just loading the touch bar buttons from the tool bar manager when the touch bar manager becomes available, so the touch bar manager would have access to the buttons array from the tool bar manager

[aminya]

@suda Would you like to transfer this repository to @atom-community, so we can expedite the process of maintenance?

[aminya]

@suda

[aminya]

@suda Any news?

[aminya]

I need to email @suda. We should ask him to give more people access to the repository.

[suda]

Hey @aminya I agree, we should transfer this package to @atom-community. Are the necessary steps for the transfer written down somewhere?