-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Public / Protected APIs #5599
Comments
cc @svenefftinge @thegecko @benoitf any opinions? |
That's a neat approach. May be hard to implement/manage/enforce automatically. |
Maybe it's for the best: it will make us think about what to expose, since we'll have to be careful about it. Having stuff private is a good default IMO, since it avoids us committing to not change it. |
APIs are not something which is marked like public/protected but abstractions which are commonly used and have well-defined stable interface. The work of analysing possible changes and deciding whether something is common and stable enough is not possible to automate. Exposing such abstractions in the code via index file or not it is another question. Right now i'm regretting of having index files at all since they tend to pull a lot of dependencies at runtime. For instance, if you want to run a test but import theia core in it then you load all reexported modules from core. It makes startup slow and bloat the memory. I would rather get rid of index files and use only absolute imports. |
First in JS there is not real private everything is accessible via As a framework developers we commit to provide customizability and at the same time do our best to keep backward compatible when it makes sense. I understand that it makes contribution hard since changes should be reviewed thoroughly, but then we don't really want to shake existing code often. Such rules also should not be applied to everything ideally some of extensions will go away to own repos and we will work on reducing and stabilizing core. |
I was speaking conceptually, to not advertise all injectable components as "you should expect everything to be backward compatible", just to give us a bit of wiggle room.
Sounds good, and with us slowly going toward 1.0, should we start discussing the few things we would break/refactor? For instance I am not too happy with the way I implemented the VS Code task/shell execution compatibility directly into the terminal extension, and I was looking at moving code around, in the end it would make it more simple to use in other components. But it will change some internal APIs...
But then does that mean that any component added to the code base should be immutable in regards to its interface? Most services can work that way, but I know that in the terminal module some confusion happened during its design, and some things didn't really fit anymore. I would suspect that some other places are like that as well. Hence why I wonder if we should see at changing what needs to be changed for 1.0?
Makes sense, but then just rename/remove a file and you break API. Also, all other packages expose everything from their root, or some specific namespace. I really don't like the idea that people should know where a class is stored in the package file tree... |
I thought for 1.0 we agreed to use VS Code extensions/Theia plugin APIs, but consider everything else as a subject to change? Right now is more important to finish VS Code extensions support. After 1.0 we can stabilize and document some abstractions from Theia core.
We are fine to break if changes are justified. Will such refactoring beneficial to support VS Code extensions? If not right now it will cause more disturbance than helping moving the project in the right direction. Please file an issue we can come back to it later after 1.0. We should get focused and have our priorities straight.
They should use proper tools to auto insert imports. |
We could follow Node.js approach, from here:
We will search for abstractions, stabilize and document them. During a review there will be an item that contributors and reviewers should ensure that documented (pubilc) APIs are never broken. Other APIs still can be broken without version 2.0. Later we can automate it with ts tooling via decorators. |
What would the decorators do/ensure? |
I meant to mark concepts as APIs with annotations and have a ts based tooling analyzing such code for breaking changes. Something like what we have with Eclipse PDE api tooling. |
In Theia everything is API regardless to its visibility. We will use js-doc annotations like There would docs provided with explaining the meaning of each annotation, how it is related to Theia versioning, should be used and reviewed. |
Some things are exposed in
index.ts
files underbrowser
/electron
/node
etc... But some others aren't (WindowService
?).I was thinking about the "Stable APIs" item on the tentative 1.0 roadmap: maybe it would be easier to consider as "public" anything exposed under these index files, and anything that a user accesses directly would be considered internal and may break without much notice?
It is simple and it makes import blocks cleaner.
The text was updated successfully, but these errors were encountered: