search

decentraland / sdk

PM repository for SDK
Apache License 2.0
4 stars 4 forks source link

Better autocomplete import experience #1004

Open nearnshaw opened 10 months ago

nearnshaw commented 10 months ago

Some people from the community seem to be having a bad experience when it comes to imports.

If I just type "Vec" into a new file, I get Vector3 and its corresponding import autocompleted. Some people apparently don't. They get offered three different suggestions for importing Vector3, including ~system/EngineApi, and ~system/RestrictedActions

One idea is to rename the objects in ~system/EngineApi so that they're slightly different

Another idea is to include empty imports in the template, if these are present it seems that the autocomplete works well (at least in the files with these import statements, not in a fresh new file). We need to evaluate if adding this imports the whole of the file or nothing.

import {} from "@dcl/sdk/ecs" import {} from "@dcl/sdk/math" ... etc

Here's a video of the current experience of some people (I have a different experience) The person running into this has: OS: macOS Sonoma 14.0 on Apple Silicon VS Code Version: 1.83.1 (Universal)

https://github.com/decentraland/sdk/assets/3507907/01aeb69b-e0f0-4bd5-bf00-e33434dd6dcd

ProteanCoding commented 5 months ago

Vector3 shouldn't even be exported out of EngineApi and RestrictedActions.

ProteanCoding commented 5 months ago

Easy enough to fix by making Vector3 (and Color3, Quaternion, etc) a class inside of EngineApi and RestrictedActions, instead of an interface.

But that would have to be done carefully, as it would break all the code that's been importing these from the wrong place so far.

As a first step, these should definitely be marked Deprecated.

leanmendoza commented 5 months ago

Easy enough to fix by making Vector3 (and Color3, Quaternion, etc) a class inside of EngineApi and RestrictedActions, instead of an interface.

But that would have to be done carefully, as it would break all the code that's been importing these from the wrong place so far.

As a first step, these should definitely be marked Deprecated.

It could be done by just marking it as @internal, but it's not so straightforward. The file api.d.ts is auto-generated, so making a conditional for those given types would be a hardcode. The simplest solution is to stop generating the api.d.ts and write the file manually. I proposed this in the past, but there was not quorum (e.g. to include documentation, I had to write https://github.com/decentraland/js-sdk-toolchain/commit/52edf1833e263e55739c75131111549de37ed6b4) and I understand why, it's harder to keeping the file up-to-date, but honestly, I think it's worth. Might we consider moving to manually writing the api.d.ts?