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

Add .js file extension to all in-project imports #10994

Merged
merged 41 commits into from
Jul 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
324c59e
add `.js` file extension to all in-project imports
phryneas Jun 20, 2023
3173f83
bundler adjustments
phryneas Jun 22, 2023
fdd53e6
remove postprocessing step
phryneas Jun 23, 2023
7071b28
fixup, script to detect build differences
phryneas Jun 23, 2023
105764b
more fixes
phryneas Jun 23, 2023
78f8f23
changeset
phryneas Jun 23, 2023
d64a965
adjust jest file resolution
phryneas Jun 23, 2023
8708fa3
fix up wrong import in `batchHttpLink`
phryneas Jun 23, 2023
da2131f
add AreTheTypesWrong check
phryneas Jun 26, 2023
933b91f
minor action changes
phryneas Jun 26, 2023
05297ce
Merge remote-tracking branch 'origin/release-3.8' into pr/fileExtensions
phryneas Jun 26, 2023
07d3410
fixup
phryneas Jun 26, 2023
6589871
use ascii format for attw
phryneas Jun 26, 2023
192f64c
update AreTheTypesWrong, use new `--pack` option
phryneas Jun 30, 2023
fdd4dce
add build comparison action
phryneas Jun 30, 2023
270cc91
omit minified comparison output
phryneas Jun 30, 2023
9353904
trigger ci
phryneas Jun 30, 2023
2c210c7
eslint configuration changes, temp custom eslint version
phryneas Jul 5, 2023
3f62df1
trigger ci
phryneas Jul 6, 2023
957e48b
give workflows distincive names?
phryneas Jul 6, 2023
45249c4
revert to latest working CI build
phryneas Jul 6, 2023
f02f584
Merge remote-tracking branch 'origin/release-3.8' into pr/fileExtensions
phryneas Jul 6, 2023
0cc6868
revert changes to workflows
phryneas Jul 6, 2023
0640eaf
clean up newly merged imports
phryneas Jul 6, 2023
d109fbe
fix command name
phryneas Jul 6, 2023
c788a07
adjust fetch-depth
phryneas Jul 6, 2023
7b79d6f
use runner temp dir
phryneas Jul 6, 2023
482f31c
dir tweaks
phryneas Jul 6, 2023
41ff620
temp file dir?
phryneas Jul 6, 2023
446f00a
fir argument
phryneas Jul 6, 2023
5ce25a8
logging
phryneas Jul 6, 2023
0458e17
more detail logging
phryneas Jul 6, 2023
9de8acc
fix binary logic
phryneas Jul 6, 2023
3bf4092
remove a bunch of logging again
phryneas Jul 6, 2023
d46c65c
disable `set`
phryneas Jul 6, 2023
b90aa50
partially reintroduce
phryneas Jul 6, 2023
86c3b76
now, to find that last command...
phryneas Jul 6, 2023
e782d42
this seems like a good compromise
phryneas Jul 6, 2023
a1c5497
Merge remote-tracking branch 'origin/release-3.8' into pr/fileExtensions
phryneas Jul 7, 2023
51e0d17
bump the TS eslint parser to deal with TS 5.1
phryneas Jul 7, 2023
a53a948
apply feedback from review
phryneas Jul 7, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .attw.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"ignoreRules": [
"false-esm",
"cjs-resolves-to-esm"
]
}
5 changes: 5 additions & 0 deletions .changeset/lemon-fans-shake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@apollo/client': minor
---

Add .js file extensions to imports in src and dist/**/*.d.ts
27 changes: 24 additions & 3 deletions .eslintrc
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"parser": "@typescript-eslint/parser",
"plugins": ["@typescript-eslint"],
"plugins": ["@typescript-eslint", "import"],
"env": {
"browser": true,
"node": true,
Expand All @@ -9,17 +9,35 @@
"parserOptions": {
"ecmaVersion": "latest"
},
"settings": {
"import/parsers": {
"@typescript-eslint/parser": [".ts", ".tsx"]
},
"import/resolver": {
"typescript": {
"alwaysTryTypes": true
}
}
},
"overrides": [
{
"files": ["**/*.ts", "**/*.tsx"],
"excludedFiles": ["**/__tests__/**/*.*"],
"rules": {
"@typescript-eslint/consistent-type-imports": ["error", {
"@typescript-eslint/consistent-type-imports": ["error", {
"prefer": "type-imports",
"disallowTypeAnnotations": false,
"fixStyle": "separate-type-imports"
}],
"@typescript-eslint/no-import-type-side-effects": "error"
"@typescript-eslint/no-import-type-side-effects": "error",
"import/extensions": [
"error",
"always",
{
"ignorePackages": true,
"checkTypeImports": true
}
]
}
},
{
Expand All @@ -31,4 +49,7 @@
}
}
],
"rules": {
"import/no-unresolved": "error"
}
}
28 changes: 28 additions & 0 deletions .github/workflows/arethetypeswrong.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
name: AreTheTypesWrong
on:
pull_request:
branches:
- main
- release-*

concurrency: ${{ github.workflow }}-${{ github.ref }}

jobs:
arethetypeswrong:
name: Are the types wrong
runs-on: ubuntu-latest
steps:
- name: Checkout repo
uses: actions/checkout@v3
- name: Setup Node.js 18.x
uses: actions/setup-node@v3
with:
node-version: 18.x
- name: Install dependencies (with cache)
uses: bahmutov/npm-install@v1

- name: Run build
run: npm run build
- name: Run AreTheTypesWrong
id: attw
run: ./node_modules/.bin/attw --format ascii --pack dist > $GITHUB_STEP_SUMMARY
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,6 @@ junit.xml
reports

esbuild-why-*.html

.yalc
yalc.lock
3 changes: 3 additions & 0 deletions config/entryPoints.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ exports.check = function (id, parentId) {
function partsAfterDist(id) {
const parts = id.split(path.sep);
const distIndex = parts.lastIndexOf("dist");
if (/^index.jsx?$/.test(parts[parts.length - 1])) {
parts.pop();
}
if (distIndex >= 0) {
return parts.slice(distIndex + 1);
}
Expand Down
1 change: 1 addition & 0 deletions config/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const defaults = {
},
],
},
resolver: "ts-jest-resolver",
};

const ignoreTSFiles = '.ts$';
Expand Down
121 changes: 4 additions & 117 deletions config/postprocessDist.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as fs from "fs";
import * as path from "path";
import resolve from "resolve";
import { distDir, eachFile, reparse, reprint } from './helpers';
import { distDir } from './helpers.ts';
import fs from 'node:fs';
import path from 'node:path';

const globalTypesFile = path.resolve(distDir, "utilities/globals/global.d.ts");
fs.writeFileSync(globalTypesFile,
Expand All @@ -10,116 +9,4 @@ fs.writeFileSync(globalTypesFile,
.filter(line => line.trim() !== 'const __DEV__: boolean;')
.join("\n"),
"utf8"
);

// The primary goal of the 'npm run resolve' script is to make ECMAScript
// modules exposed by Apollo Client easier to consume natively in web browsers,
// without bundling, and without help from package.json files. It accomplishes
// this goal by rewriting internal ./ and ../ (relative) imports to refer to a
// specific ESM module (not a directory), including its file extension. Because
// of this limited goal, this script only touches ESM modules that have .js file
// extensions, not .cjs CommonJS bundles.

// A secondary goal of this script is to enforce that any module using the
// __DEV__ global constant imports the @apollo/client/utilities/globals polyfill
// module first.

eachFile(distDir, (file, relPath) => new Promise((resolve, reject) => {
fs.readFile(file, "utf8", (error, source) => {
if (error) return reject(error);

const tr = new Transformer;
const output = tr.transform(source, file);

if (source === output) {
resolve(file);
} else {
fs.writeFile(file, output, "utf8", error => {
error ? reject(error) : resolve(file);
});
}
});
}));

import * as recast from "recast";
const n = recast.types.namedTypes;
type Node = recast.types.namedTypes.Node;

class Transformer {
absolutePaths = new Set<string>();

transform(code: string, file: string) {
const ast = reparse(code);
const transformer = this;

recast.visit(ast, {
visitImportDeclaration(path) {
this.traverse(path);
transformer.normalizeSourceString(file, path.node.source);
},

visitImportExpression(path) {
this.traverse(path);
transformer.normalizeSourceString(file, path.node.source);
},

visitExportAllDeclaration(path) {
this.traverse(path);
transformer.normalizeSourceString(file, path.node.source);
},

visitExportNamedDeclaration(path) {
this.traverse(path);
transformer.normalizeSourceString(file, path.node.source);
},
});

return reprint(ast);
}

isRelative(id: string) {
return id.startsWith("./") || id.startsWith("../");
}

normalizeSourceString(file: string, source?: Node | null) {
if (source && n.StringLiteral.check(source)) {
// We mostly only worry about normalizing _relative_ module identifiers,
// which start with a ./ or ../ and refer to other modules within the
// @apollo/client package, but we also manually normalize one non-relative
// identifier, ts-invariant/process, to prevent webpack 5 errors
// containing the phrase "failed to resolve only because it was resolved
// as fully specified," referring to webpack's resolve.fullySpecified
// option, which is apparently now true by default when the enclosing
// package's package.json file has "type": "module" (which became true for
// Apollo Client in v3.5).
if (source.value.split("/", 2).join("/") === "ts-invariant/process") {
source.value = "ts-invariant/process/index.js";
} else if (this.isRelative(source.value)) {
try {
source.value = this.normalizeId(source.value, file);
} catch (error) {
console.error(`Failed to resolve ${source.value} in ${file} with error ${error}`);
process.exit(1);
}
}
}
}

normalizeId(id: string, file: string) {
const basedir = path.dirname(file);
const absPath = resolve.sync(id, {
basedir,
extensions: [".mjs", ".js"],
packageFilter(pkg) {
return pkg.module ? {
...pkg,
main: pkg.module,
} : pkg;
},
});
this.absolutePaths.add(absPath);
const relPath = path.relative(basedir, absPath);
const relId = relPath.split(path.sep).join('/');
return this.isRelative(relId) ? relId : "./" + relId;
}
}
);
3 changes: 1 addition & 2 deletions config/processInvariants.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as fs from 'fs';
import { posix, join as osPathJoin } from 'path';
import { distDir, eachFile, reparse, reprint } from './helpers';
import { distDir, eachFile, reparse, reprint } from './helpers.ts';
import type { ExpressionKind } from 'ast-types/lib/gen/kinds';

eachFile(distDir, (file, relPath) => {
Expand Down Expand Up @@ -211,7 +211,6 @@ function transform(code: string, relativeFilePath: string) {
b.literal(false)
);
}

return node;
},
});
Expand Down
2 changes: 1 addition & 1 deletion config/rewriteSourceMaps.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as fs from "fs";
import * as path from "path";
import { distDir } from './helpers';
import { distDir } from './helpers.ts';
import glob = require("glob");

glob(`${distDir.replace(/\\/g, '/')}/**/*.js.map`, (error, files) => {
Expand Down
30 changes: 26 additions & 4 deletions config/rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import path from 'path';
import path, { resolve, dirname } from 'path';
import { promises as fs } from "fs";

import nodeResolve from '@rollup/plugin-node-resolve';
Expand Down Expand Up @@ -109,9 +109,8 @@ function prepareBundle({

return {
input: inputFile,
external(id, parentId) {
return isExternal(id, parentId, true);
},
// the external check is done by the `'externalize-dependency'` plugin
// external(id, parentId) {}
output: {
file: outputFile,
format: 'cjs',
Expand All @@ -120,6 +119,29 @@ function prepareBundle({
externalLiveBindings: false,
},
plugins: [
{
name: 'externalize-dependency',
resolveId(id, parentId) {
if (!parentId) {
return null;
}
function removeIndex(filename) {
if (filename.endsWith(`${path.sep}index.js`)) {
return filename.slice(0, -`${path.sep}index.js`.length)
}
return filename
}

const external = isExternal(id, parentId, true)
if (external) {
if (id.startsWith(".")) {
return { id: removeIndex(resolve(dirname(parentId), id)), external: true };
}
return { id: removeIndex(id), external: true };
}
return null;
}
},
extensions ? nodeResolve({ extensions }) : nodeResolve(),
{
name: "copy *.cjs to *.cjs.native.js",
Expand Down
2 changes: 2 additions & 0 deletions config/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
"esModuleInterop": true,
"sourceMap": true,
"inlineSourceMap": false,
"noEmit": true,
"allowImportingTsExtensions": true,
// This option should cause sourcesContent to be included in the
// source map JSON, so we don't have to rely on the (nonexistent)
// relative paths in the sources array, but it appears tsc does not
Expand Down
Loading