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

draft: What would it look like to enable ae-forgotten-export #275

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

stocaaro
Copy link
Member

@stocaaro stocaaro commented Jul 24, 2024

Description of changes:

TLDR: We shouldn't do this without prioritization and clear requirements. Its expensive/disruptive and would require design and review in almost any outcome state.

Overview:
ae-forgotten-export walks the graph of params and returns from the exported type surface for a package. If any of the types used to composed the public types aren't exported, then it flags them for export.

Approaches/argument:

  • We should export our entire type surface for customer consumption:
    • For Our customers may want to compose functionally any subordinant portion of our type interface. We should support our customers by exporting any types their functions may compose inputs with.
    • Against These types are already exposed, their surface is already part of our public contract and customers can access them using type utilities. Exporting them all explicitly would harden the type names themselves in the public contract which would require additional scrutiny/name-smithing around all of these types that would be exposed.
  • We should test our entire type surface for changes with API extractor:
    • For Our API's are our customer contract and any changes, expanding or contracting what is possible in our public surface needs review and scrutiny to ensure we don't break our customer experience. We can test this without exposing the types themselves by mingling the external and internal interfaces in a build target we test with api-extractor.
    • Against Its expensive in change and developer time and the payoff is safety we haven't specifically prioritized yet. This change attempts to add testing for all types. It's an enormous change that will conflict with all other work in flight / be quite noisy. Doing this doesn't solve the problem of type exports directly and doing so would still require exposing some additional types that are exposed indirectly from the types package in the data-schema package. Resolving name collisions would need to happen, raising a handful of questions about factoring.
  • We should do no new verification:
    • For Lets focus on problems our customers have today.
    • Against We are currently relying on good intentions to maintain a stable public surface. Change is going to be increasingly scary and type changes that aren't explicitely expanding the surface will all be potentially breaking.

Errors not yet resolved by this change

Analysis will use the bundled TypeScript version 5.4.2
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/builder/external.types.d.ts:34:1 - (ae-forgotten-export) The symbol "PathEntry" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/builder/external.types.d.ts:63:5 - (ae-forgotten-export) The symbol "BackendSecret" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/builder/external.types.d.ts:64:5 - (ae-forgotten-export) The symbol "VpcConfig" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/client/external.types.d.ts:8:1 - (ae-forgotten-export) The symbol "CustomSelectionSetReturnValue" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/client/external.types.d.ts:9:1 - (ae-forgotten-export) The symbol "ModelTypesClient" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/client/external.types.d.ts:9:1 - (ae-forgotten-export) The symbol "ModelTypesSSRCookies" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/client/external.types.d.ts:13:5 - (ae-forgotten-export) The symbol "AuthMode_2" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/client/external.types.d.ts:15:5 - (ae-forgotten-export) The symbol "CustomHeaders_2" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/client/external.types.d.ts:26:9 - (ae-forgotten-export) The symbol "SingularReturnValue_2" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/client/external.types.d.ts:53:1 - (ae-forgotten-export) The symbol "ModelMetaShape" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/client/external.types.d.ts:53:1 - (ae-forgotten-export) The symbol "ResolvedModel" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/client/external.types.d.ts:53:1 - (ae-forgotten-export) The symbol "IndexQueryMethodsFromIR_2" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/client/external.types.d.ts:54:5 - (ae-forgotten-export) The symbol "CreateModelInput_2" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/client/external.types.d.ts:59:5 - (ae-forgotten-export) The symbol "ModelIdentifier_2" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/client/external.types.d.ts:69:5 - (ae-forgotten-export) The symbol "ReturnValue" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/client/external.types.d.ts:75:5 - (ae-forgotten-export) The symbol "ListReturnValue_2" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/client/external.types.d.ts:76:9 - (ae-forgotten-export) The symbol "ModelFilter" needs to be exported by the entry point index.internal.d.ts�[39m
�[31mError: /amplify-api-next/packages/data-schema-types/dist/esm/client/internal.types.d.ts:124:1 - (ae-forgotten-export) The symbol "IfEquals_2" needs to be exported by the entry point index.internal.d.ts�[39m

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link

changeset-bot bot commented Jul 24, 2024

⚠️ No Changeset found

Latest commit: d72d754

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@stocaaro stocaaro changed the title darft: What would it look like to enable ae-forgotten-export draft: What would it look like to enable ae-forgotten-export Jul 24, 2024
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.

1 participant