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

Fix magic export deprecation messaging #4661

Merged
merged 6 commits into from
Nov 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 0 additions & 12 deletions .changeset/khaki-meals-prove.md

This file was deleted.

6 changes: 6 additions & 0 deletions .changeset/sharp-ears-begin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"remix": patch
"@remix-run/dev": patch
---

Importing functions and types from the `remix` package is deprecated, and all exported modules will be removed in the next major release. For more details,[see the release notes for 1.4.0](https://github.com/remix-run/remix/releases/tag/v1.4.0) where these changes were first announced.
2 changes: 2 additions & 0 deletions packages/remix-dev/compiler/compileBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { loaders } from "./loaders";
import { type CompileOptions } from "./options";
import { browserRouteModulesPlugin } from "./plugins/browserRouteModulesPlugin";
import { cssFilePlugin } from "./plugins/cssFilePlugin";
import { deprecatedRemixPackagePlugin } from "./plugins/deprecatedRemixPackagePlugin";
import { emptyModulesPlugin } from "./plugins/emptyModulesPlugin";
import { mdxPlugin } from "./plugins/mdx";
import { urlImportsPlugin } from "./plugins/urlImportsPlugin";
Expand Down Expand Up @@ -70,6 +71,7 @@ const createEsbuildConfig = (
}

let plugins = [
deprecatedRemixPackagePlugin(options.onWarning),
cssFilePlugin(options),
urlImportsPlugin(),
mdxPlugin(config),
Expand Down
2 changes: 2 additions & 0 deletions packages/remix-dev/compiler/compilerServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { type AssetsManifest } from "./assets";
import { loaders } from "./loaders";
import { type CompileOptions } from "./options";
import { cssFilePlugin } from "./plugins/cssFilePlugin";
import { deprecatedRemixPackagePlugin } from "./plugins/deprecatedRemixPackagePlugin";
import { emptyModulesPlugin } from "./plugins/emptyModulesPlugin";
import { mdxPlugin } from "./plugins/mdx";
import { serverAssetsManifestPlugin } from "./plugins/serverAssetsManifestPlugin";
Expand Down Expand Up @@ -47,6 +48,7 @@ const createEsbuildConfig = (
let isDenoRuntime = config.serverBuildTarget === "deno";

let plugins: esbuild.Plugin[] = [
deprecatedRemixPackagePlugin(options.onWarning),
cssFilePlugin(options),
urlImportsPlugin(),
mdxPlugin(config),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import path from "path";
import type { Plugin } from "esbuild";

/**
* A plugin to warn users when importing from the deprecated `remix` package
*/
export function deprecatedRemixPackagePlugin(
onWarning?: (warning: string, key: string) => void
): Plugin {
return {
name: "deprecated-remix-package",
setup(build) {
build.onResolve({ filter: /.*/ }, ({ importer, path: filePath }) => {
// Warn on deprecated imports from the remix package
if (filePath === "remix") {
let relativePath = path.relative(process.cwd(), importer);
let warningMessage =
`WARNING: All \`remix\` exports are considered deprecated as of v1.3.3. ` +
`Please change your import in "${relativePath}" to come from the respective ` +
`underlying \`@remix-run/*\` package. ` +
`Run \`npx @remix-run/dev@latest codemod replace-remix-magic-imports\` ` +
`to automatically migrate your code.`;
onWarning?.(warningMessage, importer);
}
return undefined;
});
},
};
}
51 changes: 16 additions & 35 deletions rollup.utils.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
const fs = require("fs");
const path = require("path");
const babel = require("@rollup/plugin-babel").default;
const nodeResolve = require("@rollup/plugin-node-resolve").default;
const fse = require("fs-extra");
const { camelCase, upperFirst } = require("lodash");
const copy = require("rollup-plugin-copy");
const fs = require("fs");
const fse = require("fs-extra");
const nodeResolve = require("@rollup/plugin-node-resolve").default;
const path = require("path");

const REPO_ROOT_DIR = __dirname;

Expand Down Expand Up @@ -191,28 +191,11 @@ function magicExportsPlugin({ packageName, version }) {
banner +
"\n" +
"'use strict';\n" +
"Object.defineProperty(exports, '__esModule', { value: true });\n\n";
"Object.defineProperty(exports, '__esModule', { value: true });\n";

if (magicExports.values) {
let deprecationFunctions =
// eslint-disable-next-line no-template-curly-in-string
"const getDeprecatedMessage = (symbol, packageName) => `All \\`remix\\` exports are considered deprecated as of v1.3.3. Please import \\`${symbol}\\` from \\`${packageName}\\` instead. Run \\`npx @remix-run/dev@latest codemod replace-remix-magic-imports\\` to automatically migrate your code.`;\n" +
"const warn = (fn, message) => (...args) => {\n" +
" console.warn(message);\n" +
" return fn(...args);\n" +
"};\n\n";

esmContents +=
`import * as ${moduleName} from '${packageName}';\n` +
deprecationFunctions;
esmContents += magicExports.values
.map(
(symbol) =>
`/** @deprecated Import \`${symbol}\` from \`${packageName}\` instead. */\n` +
`const ${symbol} = warn(${moduleName}.${symbol}, getDeprecatedMessage('${symbol}', '${packageName}'));\n`
)
.join("\n");
esmContents += `\nexport { ${magicExports.values.join(", ")} };\n`;
let exportList = magicExports.values.join(", ");
esmContents += `export { ${exportList} } from '${packageName}';\n`;

tsContents += `import * as ${moduleName} from '${packageName}';\n\n`;
tsContents += magicExports.values
Expand All @@ -223,17 +206,15 @@ function magicExportsPlugin({ packageName, version }) {
)
.join("\n");

cjsContents +=
`var ${moduleName} = require('${packageName}');\n` +
deprecationFunctions;
cjsContents += magicExports.values
.map(
(symbol) =>
`/** @deprecated Import \`${symbol}\` from \`${packageName}\` instead. */\n` +
`const ${symbol} = warn(${moduleName}.${symbol}, getDeprecatedMessage('${symbol}', '${packageName}'));\n` +
`exports.${symbol} = ${symbol};\n`
)
.join("\n");
let cjsModule = camelCase(packageName.slice("@remix-run/".length));
cjsContents += `var ${cjsModule} = require('${packageName}');\n`;
for (let symbol of magicExports.values) {
cjsContents +=
`Object.defineProperty(exports, '${symbol}', {\n` +
" enumerable: true,\n" +
` get: function () { return ${cjsModule}.${symbol}; }\n` +
"});\n";
}
Comment on lines +209 to +217
Copy link
Member

@MichaelDeBoey MichaelDeBoey Nov 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can keep the JSDoc deprecation here as well to have an extra IDE warning?

Suggested change
let cjsModule = camelCase(packageName.slice("@remix-run/".length));
cjsContents += `var ${cjsModule} = require('${packageName}');\n`;
for (let symbol of magicExports.values) {
cjsContents +=
`Object.defineProperty(exports, '${symbol}', {\n` +
" enumerable: true,\n" +
` get: function () { return ${cjsModule}.${symbol}; }\n` +
"});\n";
}
cjsContents += `var ${moduleName} = require('${packageName}');\n`;
cjsContents += magicExports.values
.map(
(symbol) =>
`/** @deprecated Import \`${symbol}\` from \`${packageName}\` instead. */\n` +
`exports.${symbol} = ${symbol};\n`
)
.join("\n");

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MichaelDeBoey I'm having trouble actually seeing this JSDoc warning anywhere in my VSCode IDE 🤔

If I create a fresh Remix JS project, VSCode still shows me the deprecation from our .d.ts file in node_modules/remix in my .jsx files:

Screenshot 2022-11-21 at 4 04 24 PM

If I build with this JSDoc comment included and I remove the .d.ts file, then I don't see the deprecation, and I just see the JSDoc from the internal definition of the method, not the JSDoc of the re-exported version from remix:

Screenshot 2022-11-21 at 3 55 34 PM

So for now I'm going to merge this without the CJS JSDoc updates since I can't quite prove that they're picked up. It does seem that even in non-TS projects, at least VScode will pick up our .d.ts deprecation warning.

If we can find specific cases where the JSDoc enhances the DX I'm all for a subsequent PR before we release 1.8.0 stable, but for now I want to get 1.8.0-pre.1 out so folks can help test the new @remix-run/router work inside of Remix 👍

}

if (magicExports.types) {
Expand Down