Fix module load order within `internals-js/src/specs` directory using `index.ts` module
While attempting to write new tests in the internals-js/src/specs/__tests__/ directory, I found some circularities in the import structure of the FeatureDefinition subclasses, triggered by importing a different entry point module than usual.
See for example this CI run which fails with the following error:
FAIL internals-js/src/specs/__tests__/sourceSpec.test.ts
● Test suite failed to run
TypeError: Class extends value undefined is not a constructor or null
35 | export const inaccessibleIdentity = 'https://specs.apollo.dev/inaccessible';
36 |
> 37 | export class InaccessibleSpecDefinition extends FeatureDefinition {
| ^
38 | public readonly inaccessibleLocations: DirectiveLocation[];
39 | public readonly inaccessibleDirectiveSpec: DirectiveSpecification;
40 | private readonly printedInaccessibleDefinition: string;
at Object.<anonymous> (src/specs/inaccessibleSpec.ts:37:49)
at Object.<anonymous> (src/definitions.ts:45:1)
at Object.<anonymous> (src/specs/coreSpec.ts:3:1)
at Object.<anonymous> (src/specs/sourceSpec.ts:2:1)
at Object.<anonymous> (src/specs/__tests__/sourceSpec.test.ts:1:1)
If you examine that stack trace, the crux of the problem is the inaccessibleSpec.ts module attempts to import FeatureDefinition from coreSpec.ts while the coreSpec.ts module is still being evaluated, so FeatureDefinition has not been exported yet from coreSpec.ts at the moment InaccessibleSpecDefinition tries to extend it.
I'll be the first to argue import cycles are not necessarily evil in JavaScript (ES2015+), but cycles do potentially lead to different module load order depending on which module you import first, and the internals-js/specs/*Spec.ts code is especially sensitive to load order because of synchronous use of imports like FeatureDefinition in module scope.
There are some tricks for getting extends FeatureDefinition not to complain if FeatureDefinition is undefined (until the first instance of the subclass is created), but, based on some light internet research, a more robust solution is to fix the load order among the sensitive modules using a specs/index.ts module that re-exports everything from the other *Spec.ts modules, and then make sure other code only imports from that specs/index.ts module, rather than importing from individual internals-js/specs/*Spec.ts modules. Here's a good writeup of this approach.
I'll be pushing commits incrementally so you can see the errors before/after I fix them.
🦋 Changeset detected
Latest commit: f02243e254aea8d6180b3b4110d98ff7b16cde8d
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 7 packages
| Name | Type |
|---|---|
| @apollo/federation-internals | Patch |
| @apollo/gateway | Patch |
| @apollo/composition | Patch |
| @apollo/query-planner | Patch |
| @apollo/query-graphs | Patch |
| @apollo/subgraph | Patch |
| apollo-federation-integration-testsuite | Patch |
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
Deploy Preview for apollo-federation-docs canceled.
| Name | Link |
|---|---|
| Latest commit | f02243e254aea8d6180b3b4110d98ff7b16cde8d |
| Latest deploy log | https://app.netlify.com/sites/apollo-federation-docs/deploys/65bc1d8c8803fe0008817eaf |
This pull request is automatically built and testable in CodeSandbox.
To see build info of the built libraries, click here or the icon next to each commit SHA.