Skip to content

Commit

Permalink
Fix import detection for workspaces with the same name as a local file (
Browse files Browse the repository at this point in the history
  • Loading branch information
joshuajaco committed May 7, 2024
1 parent d661070 commit 0d09ced
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 20 deletions.
37 changes: 21 additions & 16 deletions lib/rules/no-absolute-imports.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { dirname, relative } = require("path");
const {
getWorkspaces,
isSubPath,
isWorkspacePath,
getImport,
pathToImport,
} = require("../utils");
Expand All @@ -26,21 +27,25 @@ module.exports.meta = {
module.exports.create = (context) => {
const workspaces = getWorkspaces(context);
const filename = context.getFilename();
return getImport(workspaces, filename, ({ node, path, start, end }) => {
workspaces.forEach(({ package: { name }, location }) => {
if (isSubPath(location, filename) && isSubPath(name, path)) {
context.report({
node,
messageId: "noAbsoluteImports",
fix: (fixer) =>
fixer.replaceTextRange(
[start + 1, end - 1],
pathToImport(
relative(dirname(filename), path.replace(name, location)),
return getImport(
workspaces,
filename,
({ node, path, value, start, end }) => {
workspaces.forEach(({ package: { name }, location }) => {
if (isSubPath(location, filename) && isWorkspacePath(name, value)) {
context.report({
node,
messageId: "noAbsoluteImports",
fix: (fixer) =>
fixer.replaceTextRange(
[start + 1, end - 1],
pathToImport(
relative(dirname(filename), path.replace(name, location)),
),
),
),
});
}
});
});
});
}
});
},
);
};
9 changes: 7 additions & 2 deletions lib/rules/no-cross-imports.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
"use strict";

const { getWorkspaces, isSubPath, getImport } = require("../utils");
const {
getWorkspaces,
isSubPath,
isWorkspacePath,
getImport,
} = require("../utils");

module.exports.meta = {
type: "problem",
Expand Down Expand Up @@ -109,7 +114,7 @@ module.exports.create = (context) => {
.forEach(({ package: { name }, location }) => {
if (
name !== currentWorkspace.package.name &&
(isSubPath(name, value) || isSubPath(location, path))
(isWorkspacePath(name, value) || isSubPath(location, path))
) {
context.report({
node,
Expand Down
9 changes: 7 additions & 2 deletions lib/rules/require-dependency.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
"use strict";

const { getWorkspaces, isSubPath, getImport } = require("../utils");
const {
getWorkspaces,
isSubPath,
isWorkspacePath,
getImport,
} = require("../utils");

module.exports.meta = {
type: "problem",
Expand Down Expand Up @@ -32,7 +37,7 @@ module.exports.create = (context) => {

if (
name !== currentWorkspace.package.name &&
(isSubPath(name, value) || isSubPath(location, path)) &&
(isWorkspacePath(name, value) || isSubPath(location, path)) &&
!Object.keys(dependencies).includes(name) &&
!Object.keys(peerDependencies).includes(name) &&
!Object.keys(optionalDependencies).includes(name) &&
Expand Down
6 changes: 6 additions & 0 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ const isSubPath = (parent, path) => {
return relativePath.split(sep)[0] !== ".." && !isAbsolute(relativePath);
};

const isWorkspacePath = (name, path) =>
path === name ||
path.startsWith(name + sep) ||
path.startsWith(name + posix.sep);

const resolvePath = (parent, path) => {
if (path[0] !== ".") return path;

Expand Down Expand Up @@ -90,6 +95,7 @@ const getWorkspaces = (context) => {

module.exports = {
isSubPath,
isWorkspacePath,
getImport,
pathToImport,
getWorkspaces,
Expand Down
6 changes: 6 additions & 0 deletions tests/mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ module.exports.findWorkspacesMock = () => [
name: "@test/third-workspace",
},
},
{
location: "/test/no-npm-scope-workspace",
package: {
name: "no-npm-scope-workspace",
},
},
{
location: "/test/peer-dependencies",
package: {
Expand Down
4 changes: 4 additions & 0 deletions tests/rules/no-absolute-imports.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ describe("no-absolute-imports", () => {
code: "import workspace from '@test/workspace';",
filename: "/some/path.js",
},
{
filename: "/test/no-npm-scope-workspace/index.js",
code: "import './no-npm-scope-workspace';",
},
],

invalid: [
Expand Down
4 changes: 4 additions & 0 deletions tests/rules/no-cross-imports.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ describe("no-cross-imports", () => {
filename: "/test/scope/workspace/file.js",
code: "import '@test/shared-in-scope';",
},
{
filename: "/test/workspace/index.js",
code: "import './no-npm-scope-workspace';",
},
],

invalid: [
Expand Down
4 changes: 4 additions & 0 deletions tests/rules/no-relative-imports.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ describe("no-relative-imports", () => {
filename: "/some/file.js",
code: "import '../test/workspace';",
},
{
filename: "/test/workspace/index.js",
code: "import './no-npm-scope-workspace';",
},
],
invalid: [
{
Expand Down
4 changes: 4 additions & 0 deletions tests/rules/require-dependency.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ describe("require-dependency", () => {
filename: "/test/optional-dependencies/test.js",
code: "import '@test/optional-workspace'",
},
{
filename: "/test/workspace/index.js",
code: "import './no-npm-scope-workspace';",
},
],
invalid: [
{
Expand Down

0 comments on commit 0d09ced

Please sign in to comment.