Skip to content
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

chore(terra-draw): Export basic types that appear in arguments and return values #440

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ciscorn
Copy link

@ciscorn ciscorn commented Jan 22, 2025

Description of Changes

This PR exports some basic types that appear in the arguments and return types of the user-facing interfaces.

Link to Issue

Resolves #439

PR Checklist

  • The PR title follows the conventional commit standard
  • There is a associated GitHub issue
  • If I have added significant code changes, there are relevant tests
  • If there are behaviour changes these are documented

@ciscorn ciscorn changed the title Export basic types that appear in arguments and return values chore(terra-draw): Export basic types that appear in arguments and return values Jan 22, 2025
@neodescis
Copy link

Should we also export TerraDrawBaseSelectMode? HexColor?

@ciscorn
Copy link
Author

ciscorn commented Jan 23, 2025

@neodescis HexColor is already exported, and TerraDrawBaseSelectMode probably doesn't appear in the public API.

@neodescis
Copy link

neodescis commented Jan 23, 2025

Right you are about HexColor. I think I was remembering HexColorStyling which does not appear to be exported. As for TerraDrawBaseSelectMode, the documentation states that you should be able to provide your own select mode. The private getSelectMode() method casts to TerraDrawBaseSelectMode. This function is internal, but it is called by public methods that rely on the methods defined in that base class. So, it seems like it should be exported in support of custom select modes; otherwise you have to hope a custom select mode's methods line up with it without type support.

@ciscorn
Copy link
Author

ciscorn commented Jan 24, 2025

@neodescis
I agree with your perspective. I’ve added TerraDrawBaseSelectMode to the export list in this PR.

@JamesLMilner
Copy link
Owner

Hey, sorry folks not ignoring this, just need a bit of free time to give it proper time to think it through - on surface makes a lot of sense however. One aspect to think about is that there is an attempt to expose types for extending Terra Draw in TerraDrawExtend. This might have been a mistake that adds extra confusion but until v2 we can't change it.

@JinIgarashi
Copy link
Contributor

JinIgarashi commented Feb 2, 2025

I think this PR is also related to #396. I found terra-draw supports types when moduleResolution = node in tsconfig.json. even you export additional types from terra-draw, the types will never work unless terra-draw supports other moduleResolution like nodenext for modern nodejs application. If we change moduleResolution to node, it can work, but it does not make sense to use node for most of applications.

@JinIgarashi
Copy link
Contributor

I realized FeatureID type should be exported as well. change event of terradraw uses FeatureID type, there is an issue of defining a callback function for change event.

For moduleResolution setting, I found moduleResolution: bundler can work with terra-draw types. but it does not work with nodeNext

@ciscorn ciscorn force-pushed the export-basic-types branch from 0d2139c to 5477d19 Compare February 2, 2025 12:42
@ciscorn
Copy link
Author

ciscorn commented Feb 2, 2025

@JinIgarashi Thanks! As for FeatureID, I believe it has already been exported in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Chore] Export basic types appear in arguments and return types
4 participants