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

Flatten nested imports and add deprecation notice #8901

Closed
4 tasks done
Tracked by #8870
joshblack opened this issue Jun 14, 2021 · 3 comments · Fixed by #9008
Closed
4 tasks done
Tracked by #8870

Flatten nested imports and add deprecation notice #8901

joshblack opened this issue Jun 14, 2021 · 3 comments · Fixed by #9008
Labels

Comments

@joshblack
Copy link
Contributor

joshblack commented Jun 14, 2021

Discussion: #8642

This issue is for tackling the normalization of our export strategy. Currently, we export a mixed of named exports and exports only available as fields on these exports. For example: TextInput and TextInput.ControlledPasswordInput.

With this change, we would like all exports to be available as named exports. To get there, we will need to:

  • Identify all components that are exported as a field on another component
  • Determine their export name in the normalized approach
  • Add each component as an export using the normalized approach
  • Add a deprecation notice to the old import with a message instructing teams on how to migrate to the new import
@joshblack
Copy link
Contributor Author

For deprecation notices, we may need to do something like:

function deprecateOnField(object, field, Component) {
  Object.defineProperty(object, field, {
    enumerable: true,
    get() {
      // Add deprecation notice here
      return Component;
    },
  });
}

// ...

deprecateOnField(TextInput, 'ControlledPasswordInput', ControlledPasswordInput);

@tay1orjones
Copy link
Member

tay1orjones commented Jun 18, 2021

I think these are the only components exported as a field on another component:

  • TextInput.ControlledPasswordInput
  • TextInput.PasswordInput
  • MultiSelect.Filterable
  • ErrorBoundaryContext.Consumer - I don't think this one we really have control over or would want to change
  • ErrorBoundaryContext.Provider - Same here

With proposed export name in the normalized approach

Current Proposed
TextInput.ControlledPasswordInput ControlledPasswordInput
TextInput.PasswordInput PasswordInput
MultiSelect.Filterable FilterableMultiSelect
ErrorBoundaryContext.Consumer no change
ErrorBoundaryContext.Provider no change

@joshblack
Copy link
Contributor Author

@tay1orjones I think that tracks!

For the ErrorBoundary stuff, I think you're right. That should stay as consumer/provider

@kodiakhq kodiakhq bot closed this as completed in #9008 Jul 23, 2021
kodiakhq bot added a commit that referenced this issue Jul 23, 2021
* feat(project): add flat exports for namespaced components, #8901

* feat(exports): add deprecation notice for namespaced exports

* chore: update snaps

* test: ignore field deprecation warnings

* fix(exports): properly mock deprecateFieldOnObject, add name fallback

* test(multiselect): correct import paths, add deprecation mock to tests

Co-authored-by: Scott Strubberg <sstrubberg@protonmail.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants