Skip to content

Add additional types to features #87

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

Closed
cinxmo opened this issue Nov 1, 2023 · 2 comments
Closed

Add additional types to features #87

cinxmo opened this issue Nov 1, 2023 · 2 comments
Labels
enhancement New feature or request good first issue Good for newcomers question Further information is requested

Comments

@cinxmo
Copy link
Contributor

cinxmo commented Nov 1, 2023

Once #40 merges, all 3 "types" will be considered file attachments:

  • local imports
  • local fetches
  • actual file attachments (created when we call FileAttachments(...) within a cell)

This causes an issue because import files would be available within the inputs of a certain cell, but it is not needed. We only promoted local imports to file attachments because we want them copied over during the build step.

The proposed solution is to define other feature types (LocalFetch, LocalImport) and modify the code to explicitly check for certain types

@cinxmo cinxmo mentioned this issue Nov 2, 2023
1 task
@cinxmo cinxmo added enhancement New feature or request good first issue Good for newcomers labels Nov 2, 2023
@mbostock
Copy link
Member

mbostock commented Nov 4, 2023

This causes an issue because import files would be available within the inputs of a certain cell, but it is not needed.

Why is this an issue? What is the harm in allowing the files to be fetched, if desired, by implicitly promoting them to file attachments? The code must be included with the built site, so there’s no reason to prevent them from being loaded if desired.

@mbostock mbostock added the question Further information is requested label Nov 4, 2023
@mbostock
Copy link
Member

mbostock commented Nov 9, 2023

I closed this fixed by #120, which made it so imports are no longer promoted to file attachments, and added type safety. We could something similar for local fetches, but I’m not sure anything is needed. PR is welcome though!

@mbostock mbostock closed this as completed Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants