Skip to content

Commit

Permalink
Fix imports for moduleResolution: nodenext TS users (v4) (#6731)
Browse files Browse the repository at this point in the history
When installing @apollo/server v4 in a TS project using "moduleResolution": "nodenext",
TS would throw errors about how Apollo Server's imports need to use extensions. 

Specifically, we omitted extensions from all type-only imports since they didn't seem to
be needed, but it's not hurting anything to add them and some use cases require it.

Add a new smoke test for this use case which will ensure extensions are used in all
source code files, even for type-only imports.

Fixes #6730
  • Loading branch information
trevor-scheer committed Aug 24, 2022
1 parent 686a30c commit 9fc23f7
Show file tree
Hide file tree
Showing 32 changed files with 133 additions and 42 deletions.
5 changes: 5 additions & 0 deletions .changeset/old-papayas-switch.md
@@ -0,0 +1,5 @@
---
"@apollo/server": patch
---

Use extensions for all imports to accommodate TS users using moduleResolution: "nodenext"
1 change: 1 addition & 0 deletions cspell-dict.txt
Expand Up @@ -115,6 +115,7 @@ mygraph
myvariant
namespacing
nanos
nodenext
npmrc
nsolid
oneof
Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -17,7 +17,7 @@
"test:clean": "jest --clearCache",
"test:watch": "jest --verbose --watchAll",
"test:smoke": "npm run test:smoke:prepare && npm run test:smoke:run",
"test:smoke:prepare": "npm run compile && smoke-test/prepare.sh",
"test:smoke:prepare": "smoke-test/prepare.sh",
"test:smoke:run": "smoke-test/smoke-test.sh",
"testonly": "npm test",
"test:ci": "npm test -- --ci --maxWorkers=2 --reporters=default --reporters=jest-junit",
Expand Down
4 changes: 2 additions & 2 deletions packages/server/src/ApolloServer.ts
Expand Up @@ -43,7 +43,7 @@ import type {
HTTPGraphQLHead,
ContextThunk,
GraphQLRequestContext,
} from './externalTypes';
} from './externalTypes/index.js';
import { runPotentiallyBatchedHttpQuery } from './httpBatching.js';
import { InternalPluginId, pluginIsInternal } from './internalPlugin.js';
import {
Expand All @@ -62,7 +62,7 @@ import { SchemaManager } from './utils/schemaManager.js';
import { isDefined } from './utils/isDefined.js';
import { UnreachableCaseError } from './utils/UnreachableCaseError.js';
import type { WithRequired } from '@apollo/utils.withrequired';
import type { ApolloServerOptionsWithStaticSchema } from './externalTypes/constructor';
import type { ApolloServerOptionsWithStaticSchema } from './externalTypes/constructor.js';
import type { GatewayExecutor } from '@apollo/server-gateway-interface';

const NoIntrospection = (context: ValidationContext) => ({
Expand Down
2 changes: 1 addition & 1 deletion packages/server/src/determineApolloConfig.ts
@@ -1,5 +1,5 @@
import { createHash } from '@apollo/utils.createhash';
import type { ApolloConfig, ApolloConfigInput } from './externalTypes';
import type { ApolloConfig, ApolloConfigInput } from './externalTypes/index.js';

// This function combines the `apollo` constructor argument and some environment
// variables to come up with a full ApolloConfig.
Expand Down
4 changes: 2 additions & 2 deletions packages/server/src/express4/index.ts
@@ -1,11 +1,11 @@
import type { WithRequired } from '@apollo/utils.withrequired';
import type express from 'express';
import type { ApolloServer } from '..';
import type { ApolloServer } from '../index.js';
import type {
BaseContext,
ContextFunction,
HTTPGraphQLRequest,
} from '../externalTypes';
} from '../externalTypes/index.js';
import { parse as urlParse } from 'url';

export interface ExpressContextFunctionArgument {
Expand Down
4 changes: 2 additions & 2 deletions packages/server/src/externalTypes/constructor.ts
Expand Up @@ -10,8 +10,8 @@ import type {
} from 'graphql';
import type { KeyValueCache } from '@apollo/utils.keyvaluecache';
import type { GatewayInterface } from '@apollo/server-gateway-interface';
import type { BaseContext } from '.';
import type { ApolloServerPlugin } from './plugins';
import type { ApolloServerPlugin } from './plugins.js';
import type { BaseContext } from './index.js';

export type DocumentStore = KeyValueCache<DocumentNode>;

Expand Down
4 changes: 2 additions & 2 deletions packages/server/src/externalTypes/graphql.ts
Expand Up @@ -7,8 +7,8 @@ import type {
OperationDefinitionNode,
} from 'graphql';
import type { CachePolicy } from '@apollo/cache-control-types';
import type { BaseContext } from './context';
import type { HTTPGraphQLHead, HTTPGraphQLRequest } from './http';
import type { BaseContext } from './context.js';
import type { HTTPGraphQLHead, HTTPGraphQLRequest } from './http.js';
import type { Logger } from '@apollo/utils.logger';
import type { KeyValueCache } from '@apollo/utils.keyvaluecache';

Expand Down
8 changes: 4 additions & 4 deletions packages/server/src/externalTypes/plugins.ts
@@ -1,9 +1,9 @@
import type { KeyValueCache } from '@apollo/utils.keyvaluecache';
import type { Logger } from '@apollo/utils.logger';
import type { GraphQLResolveInfo, GraphQLSchema } from 'graphql';
import type { ApolloConfig } from './constructor';
import type { BaseContext } from './context';
import type { GraphQLRequestContext, GraphQLResponse } from './graphql';
import type { ApolloConfig } from './constructor.js';
import type { BaseContext } from './context.js';
import type { GraphQLRequestContext, GraphQLResponse } from './graphql.js';
import type {
GraphQLRequestContextDidEncounterErrors,
GraphQLRequestContextDidResolveOperation,
Expand All @@ -13,7 +13,7 @@ import type {
GraphQLRequestContextResponseForOperation,
GraphQLRequestContextValidationDidStart,
GraphQLRequestContextWillSendResponse,
} from './requestPipeline';
} from './requestPipeline.js';

export interface GraphQLServerContext {
readonly logger: Logger;
Expand Down
4 changes: 2 additions & 2 deletions packages/server/src/externalTypes/requestPipeline.ts
@@ -1,6 +1,6 @@
import type { WithRequired } from '@apollo/utils.withrequired';
import type { BaseContext } from './context';
import type { GraphQLRequestContext } from './graphql';
import type { BaseContext } from './context.js';
import type { GraphQLRequestContext } from './graphql.js';

export type GraphQLRequestContextDidResolveSource<
TContext extends BaseContext,
Expand Down
2 changes: 1 addition & 1 deletion packages/server/src/httpBatching.ts
Expand Up @@ -2,7 +2,7 @@ import type {
BaseContext,
HTTPGraphQLRequest,
HTTPGraphQLResponse,
} from './externalTypes';
} from './externalTypes/index.js';
import type {
ApolloServer,
ApolloServerInternals,
Expand Down
2 changes: 1 addition & 1 deletion packages/server/src/internalPlugin.ts
@@ -1,4 +1,4 @@
import type { BaseContext, ApolloServerPlugin } from './externalTypes';
import type { BaseContext, ApolloServerPlugin } from './externalTypes/index.js';

// This file's exports should not be exported from the overall
// @apollo/server module.
Expand Down
2 changes: 1 addition & 1 deletion packages/server/src/plugin/cacheControl/index.ts
@@ -1,4 +1,4 @@
import type { ApolloServerPlugin } from '../../externalTypes';
import type { ApolloServerPlugin } from '../../externalTypes/index.js';
import {
DirectiveNode,
getNamedType,
Expand Down
4 changes: 2 additions & 2 deletions packages/server/src/plugin/disabled/index.ts
@@ -1,10 +1,10 @@
// TODO(AS4): Document this file
// TODO(AS4): Decide where it is imported from.
import type { BaseContext, ApolloServerPlugin } from '../..';
import type { BaseContext, ApolloServerPlugin } from '../../index.js';
import type {
InternalApolloServerPlugin,
InternalPluginId,
} from '../../internalPlugin';
} from '../../internalPlugin.js';

function disabledPlugin(id: InternalPluginId): ApolloServerPlugin {
const plugin: InternalApolloServerPlugin<BaseContext> = {
Expand Down
2 changes: 1 addition & 1 deletion packages/server/src/plugin/drainHttpServer/index.ts
@@ -1,5 +1,5 @@
import type http from 'http';
import type { ApolloServerPlugin } from '../../externalTypes';
import type { ApolloServerPlugin } from '../../externalTypes/index.js';
import { Stopper } from './stoppable.js';

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/server/src/plugin/inlineTrace/index.ts
Expand Up @@ -3,7 +3,7 @@ import { TraceTreeBuilder } from '../traceTreeBuilder.js';
import type { SendErrorsOptions } from '../usageReporting/index.js';
import { internalPlugin } from '../../internalPlugin.js';
import { schemaIsFederated } from '../schemaIsFederated.js';
import type { ApolloServerPlugin } from '../../externalTypes';
import type { ApolloServerPlugin } from '../../externalTypes/index.js';

export interface ApolloServerPluginInlineTraceOptions {
/**
Expand Down
9 changes: 6 additions & 3 deletions packages/server/src/plugin/landingPage/default/index.ts
@@ -1,10 +1,13 @@
import type { ApolloServerPlugin, BaseContext } from '../../../externalTypes';
import type { ImplicitlyInstallablePlugin } from '../../../ApolloServer';
import type {
ApolloServerPlugin,
BaseContext,
} from '../../../externalTypes/index.js';
import type { ImplicitlyInstallablePlugin } from '../../../ApolloServer.js';
import type {
ApolloServerPluginLandingPageLocalDefaultOptions,
ApolloServerPluginLandingPageProductionDefaultOptions,
LandingPageConfig,
} from './types';
} from './types.js';
import {
getEmbeddedExplorerHTML,
getEmbeddedSandboxHTML,
Expand Down
4 changes: 2 additions & 2 deletions packages/server/src/plugin/schemaReporting/index.ts
Expand Up @@ -4,8 +4,8 @@ import { v4 as uuidv4 } from 'uuid';
import { printSchema, validateSchema, buildSchema } from 'graphql';
import { SchemaReporter } from './schemaReporter.js';
import { schemaIsFederated } from '../schemaIsFederated.js';
import type { SchemaReport } from './generated/operations';
import type { ApolloServerPlugin } from '../../externalTypes';
import type { SchemaReport } from './generated/operations.js';
import type { ApolloServerPlugin } from '../../externalTypes/index.js';
import type { Fetcher } from '@apollo/utils.fetcher';
import { packageVersion } from '../../generated/packageVersion.js';
import { computeCoreSchemaHash } from '../../utils/computeCoreSchemaHash.js';
Expand Down
@@ -1,5 +1,5 @@
import fetch from 'node-fetch';
import type { GraphQLRequest } from '../../externalTypes';
import type { GraphQLRequest } from '../../externalTypes/index.js';
import type { Logger } from '@apollo/utils.logger';
import type {
SchemaReport,
Expand Down
2 changes: 1 addition & 1 deletion packages/server/src/plugin/usageReporting/options.ts
Expand Up @@ -4,7 +4,7 @@ import type {
GraphQLRequestContext,
GraphQLRequestContextWillSendResponse,
BaseContext,
} from '../../externalTypes';
} from '../../externalTypes/index.js';
import type { Logger } from '@apollo/utils.logger';
import type { Trace } from '@apollo/usage-reporting-protobuf';
import type { Fetcher } from '@apollo/utils.fetcher';
Expand Down
6 changes: 3 additions & 3 deletions packages/server/src/plugin/usageReporting/plugin.ts
Expand Up @@ -19,9 +19,9 @@ import type {
GraphQLRequestContextWillSendResponse,
GraphQLRequestListener,
GraphQLServerListener,
} from '../../externalTypes';
} from '../../externalTypes/index.js';
import { internalPlugin } from '../../internalPlugin.js';
import type { HeaderMap } from '../../runHttpQuery';
import type { HeaderMap } from '../../runHttpQuery.js';
import { dateToProtoTimestamp, TraceTreeBuilder } from '../traceTreeBuilder.js';
import { defaultSendOperationsAsTrace } from './defaultSendOperationsAsTrace.js';
import {
Expand All @@ -32,7 +32,7 @@ import {
import type {
ApolloServerPluginUsageReportingOptions,
SendValuesBaseOptions,
} from './options';
} from './options.js';
import { OurReport } from './stats.js';
import { makeTraceDetails } from './traceDetails.js';
import { packageVersion } from '../../generated/packageVersion.js';
Expand Down
2 changes: 1 addition & 1 deletion packages/server/src/plugin/usageReporting/traceDetails.ts
@@ -1,5 +1,5 @@
import { Trace } from '@apollo/usage-reporting-protobuf';
import type { VariableValueOptions } from './options';
import type { VariableValueOptions } from './options.js';

// Creates trace details from request variables, given a specification for modifying
// values of private or sensitive variables.
Expand Down
4 changes: 2 additions & 2 deletions packages/server/src/requestPipeline.ts
Expand Up @@ -42,7 +42,7 @@ import type {
GraphQLRequestExecutionListener,
BaseContext,
HTTPGraphQLHead,
} from './externalTypes';
} from './externalTypes/index.js';

import {
invokeDidStartHook,
Expand All @@ -57,7 +57,7 @@ import type {
ApolloServer,
ApolloServerInternals,
SchemaDerivedData,
} from './ApolloServer';
} from './ApolloServer.js';
import { isDefined } from './utils/isDefined.js';

export const APQ_CACHE_PREFIX = 'apq:';
Expand Down
2 changes: 1 addition & 1 deletion packages/server/src/runHttpQuery.ts
Expand Up @@ -4,7 +4,7 @@ import type {
HTTPGraphQLHead,
HTTPGraphQLRequest,
HTTPGraphQLResponse,
} from './externalTypes';
} from './externalTypes/index.js';
import {
ApolloServer,
ApolloServerInternals,
Expand Down
4 changes: 2 additions & 2 deletions packages/server/src/standalone/index.ts
Expand Up @@ -4,9 +4,9 @@ import cors from 'cors';
import express from 'express';
import http, { IncomingMessage, ServerResponse } from 'http';
import type { ListenOptions } from 'net';
import type { ApolloServer } from '../ApolloServer';
import type { ApolloServer } from '../ApolloServer.js';
import { expressMiddleware } from '../express4/index.js';
import type { BaseContext, ContextFunction } from '../externalTypes';
import type { BaseContext, ContextFunction } from '../externalTypes/index.js';
import { ApolloServerPluginDrainHttpServer } from '../plugin/drainHttpServer/index.js';
import { urlForHttpServer } from '../utils/urlForHttpServer.js';

Expand Down
2 changes: 1 addition & 1 deletion packages/server/src/utils/schemaInstrumentation.ts
Expand Up @@ -9,7 +9,7 @@ import {
import type {
BaseContext,
GraphQLRequestExecutionListener,
} from '../externalTypes';
} from '../externalTypes/index.js';

export const symbolExecutionDispatcherWillResolveField = Symbol(
'apolloServerExecutionDispatcherWillResolveField',
Expand Down
7 changes: 5 additions & 2 deletions packages/server/src/utils/schemaManager.ts
Expand Up @@ -5,8 +5,11 @@ import type {
GatewayInterface,
GatewayUnsubscriber,
} from '@apollo/server-gateway-interface';
import type { SchemaDerivedData } from '../ApolloServer';
import type { ApolloConfig, GraphQLSchemaContext } from '../externalTypes';
import type { SchemaDerivedData } from '../ApolloServer.js';
import type {
ApolloConfig,
GraphQLSchemaContext,
} from '../externalTypes/index.js';

type SchemaDerivedDataProvider = (
apiSchema: GraphQLSchema,
Expand Down
4 changes: 4 additions & 0 deletions smoke-test/nodenext/package.json
@@ -0,0 +1,4 @@
{
"type": "module",
"module": "./dist/smoke-test.js"
}
53 changes: 53 additions & 0 deletions smoke-test/nodenext/src/smoke-test.ts
@@ -0,0 +1,53 @@
import { ApolloServer } from '@apollo/server';
import { startStandaloneServer } from '@apollo/server/standalone';
import fetch from 'make-fetch-happen';
import assert from 'assert';

async function validateAllImports() {
await import('@apollo/server');
await import('@apollo/server/plugin/cacheControl');
await import('@apollo/server/plugin/disabled');
await import('@apollo/server/plugin/drainHttpServer');
await import('@apollo/server/plugin/inlineTrace');
await import('@apollo/server/plugin/landingPage/default');
await import('@apollo/server/plugin/schemaReporting');
await import('@apollo/server/plugin/usageReporting');
await import('@apollo/server/standalone');
}

async function smokeTest() {
await validateAllImports();

const s = new ApolloServer({
typeDefs: 'type Query {hello:String}',
resolvers: {
Query: {
hello() {
return 'world';
},
},
},
});
const { url } = await startStandaloneServer(s, { listen: { port: 0 } });

const response = await fetch(url, {
method: 'POST',
headers: { 'content-type': 'application/json' },
body: JSON.stringify({ query: '{hello}' }),
});
const body = await response.json();

assert.strictEqual(body.data.hello, 'world');

await s.stop();
}

smokeTest()
.then(() => {
console.log('TS-NODENEXT smoke test passed!');
process.exit(0);
})
.catch((e) => {
console.error(e);
process.exit(1);
});
11 changes: 11 additions & 0 deletions smoke-test/nodenext/tsconfig.json
@@ -0,0 +1,11 @@
{
"compilerOptions": {
"rootDir": "./src",
"outDir": "./dist",
"module": "NodeNext",
"moduleResolution": "nodenext",
"target": "es2020",
"lib": ["es2020"],
"types": ["node"]
}
}
2 changes: 2 additions & 0 deletions smoke-test/prepare.sh
Expand Up @@ -5,6 +5,8 @@ set -x

TARBALL_DIR=$(mktemp -d)

# Ensure build is current.
npm run compile
# Make tarballs of all packages.
npm pack --pack-destination="$TARBALL_DIR" --workspaces=true

Expand Down
9 changes: 9 additions & 0 deletions smoke-test/smoke-test.sh
Expand Up @@ -23,6 +23,15 @@ grep 'function createApplication' "$ROLLUP_OUT_DIR"/bundle.mjs
# ... and that the one that doesn't, doesn't.
! grep 'function createApplication' "$ROLLUP_OUT_DIR"/bundle-no-express.mjs

# Nodenext needs its own special folder - for this test to exercise the case
# we're after, we need a package.json using type: module and a bleeding edge
# tsconfig using moduleResolution: nodenext. Let's run it before the others
# since this is the "pickiest" of the tests.
cd nodenext
tsc --build .
node ./dist/smoke-test.js
cd ..

# Ensure basic TypeScript builds work.
tsc --build tsconfig.{esm,cjs}.json
node generated/tsc/smoke-test.cjs
Expand Down

0 comments on commit 9fc23f7

Please sign in to comment.