Skip to content

Commit

Permalink
Process ESM .js modules in ./dist with Rollup, again. (#6657)
Browse files Browse the repository at this point in the history
This partially reverts commit 627bd1b
from PR #6656.

PR #6656 removed a function called `prepareESM`, which was no longer
needed for the sake of running `rollup-plugin-invariant`, but which had
another benefit: Rollup would replace imports/exports like

  // Example exports from ./dist/index.js:
  export * from './core'
  export * from './react'

with their fully-expanded (and file-extensioned) equivalents:

  ...
  export { makeVar } from './cache/inmemory/reactiveVars.js';
  export { defaultDataIdFromObject } from './cache/inmemory/policies.js';
  export { InMemoryCache } from './cache/inmemory/inMemoryCache.js';
  export { ApolloClient } from './core/ApolloClient.js';
  ...

Although this may look like more code, it effectively saves the
bundler/browser work by reducing indirection, and including the .js file
extensions makes this code a little more browser- and Rollup-friendly
(that is, you shouldn't have to use @rollup/plugin-node-resolve to bundle
@apollo/client with Rollup).

The expanded imports/exports are also closer to the ESM code shipped with
@apollo/[email protected], so restoring this behavior helps with the goal of
keeping the changes in #6656 backwards-compatible for v3.1.0.

Note that the CommonJS bundles used by Node.js are built before this step,
so this expansion of imports does not affect Node.js.
  • Loading branch information
benjamn authored Jul 21, 2020
1 parent a1855ff commit faa64b3
Showing 1 changed file with 25 additions and 0 deletions.
25 changes: 25 additions & 0 deletions config/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,30 @@ function prepareBundle({
};
}

// Resolve indirect imports and exports to the original exporting module,
// so that more imports are explicitly named (fewer *s), and all source
// module identifiers have file extensions.
function resolveESMImportsAndFileExtensions(input, outputDir) {

This comment has been minimized.

Copy link
@henryqdineen

henryqdineen Jul 23, 2020

Contributor

@benjamn I tried out 3.1.0 and we are seeing a bundle size regression in our rollup bundle. Here is a snippet from dist/core/index.js before and after

export { ApolloClient, } from './ApolloClient';
export { ObservableQuery, } from './ObservableQuery';
export { NetworkStatus } from './networkStatus';
export * from './types';
export { isApolloError, ApolloError } from '../errors';
import '../utilities/graphql/directives.js';
import '../utilities/graphql/fragments.js';
export { isReference, makeReference } from '../utilities/graphql/storeUtils.js';
import '../utilities/graphql/getFromAST.js';
import '../utilities/graphql/transform.js';
export { default as Observable } from 'zen-observable';
import '../utilities/common/mergeDeep.js';
import '../utilities/common/cloneDeep.js';
import '../utilities/common/maybeDeepFreeze.js';

I don't think rollup is able to treeshake the imports with no specifiers. Please let me know if you need any more info if there are any workarounds.

This comment has been minimized.

Copy link
@henryqdineen

henryqdineen Jul 23, 2020

Contributor

Moved to an issue: #6687

return {
input,
external(id) {
return externalPackages.has(id);
},
output: {
dir: outputDir,
format: 'esm',
sourcemap: true,
},
// By setting preserveModules to true, we're making sure Rollup
// doesn't attempt to create a single combined ESM bundle with the
// final result of running this job.
preserveModules: true,
plugins: [
nodeResolve(),
],
};
}

export default [
...entryPoints.map(prepareBundle),
// Convert the ESM entry point to a single CJS bundle.
Expand All @@ -104,4 +128,5 @@ export default [
prepareCJSMinified(
'./dist/apollo-client.cjs.js',
),
resolveESMImportsAndFileExtensions(packageJson.module, distDir),
];

0 comments on commit faa64b3

Please sign in to comment.