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!: correctly detect if file is outside base path on Windows #59

Merged
merged 26 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
2b5a704
fix: correctly detect is file is outside base path on Windows
fasttime Jun 16, 2024
8e3ac11
`toNamespacedPath` fallback to identity, use `process.cwd`, improve c…
fasttime Jun 23, 2024
47386e1
Add generic `toNamespacedPath` polyfill
fasttime Jun 26, 2024
efacd06
link to Node.js license
fasttime Jun 28, 2024
ad9d95e
select `path` implementations depending on base path
fasttime Aug 18, 2024
f47909a
fix for `isDirectoryIgnored` when no base path specified
fasttime Aug 18, 2024
90ed296
simplify selection of path-handling implementations
fasttime Aug 19, 2024
f4dd522
download `@std/path` package from tarball URL
fasttime Sep 4, 2024
0cad743
use pre-bundled versions of `@jsr/std__path`
fasttime Sep 8, 2024
face4eb
update jsr.json in `config-array`
fasttime Sep 8, 2024
ae5b89a
fix unit tests in `compat`
fasttime Sep 8, 2024
6140189
throw an error if a path is not absolute
fasttime Sep 15, 2024
c8d62a3
allow relative paths
fasttime Sep 27, 2024
ba5056e
don't resolve relative paths in tests
fasttime Sep 28, 2024
6eaced9
default `basePath` to `"/"`
fasttime Oct 13, 2024
6fd825b
revert changes in `compat` tests
fasttime Oct 13, 2024
98efade
use private fields
fasttime Oct 17, 2024
3b90297
resolve paths to `@jsr/std__path` dependency exports
fasttime Oct 17, 2024
882c3ec
use non-capturing group
fasttime Oct 19, 2024
606adcf
fix typos
fasttime Oct 19, 2024
ab61572
annotate types of private fields
fasttime Oct 19, 2024
d72dd7d
add dot
fasttime Oct 19, 2024
4ed3c1c
disallow empty strings for `basePath`
fasttime Oct 23, 2024
8c30779
update docs
fasttime Oct 23, 2024
fb712b1
improve basePath validation
fasttime Oct 24, 2024
949b364
remove unnecessary `async` keword from test functions
fasttime Oct 24, 2024
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
1 change: 1 addition & 0 deletions .npmrc
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
package-lock = false
@jsr:registry=https://npm.jsr.io
Copy link
Member

Choose a reason for hiding this comment

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

Would end users need to add this to their .npmrc files?

Copy link
Member

Choose a reason for hiding this comment

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

I've tried using npm pack-ed version of config-array from this branch in eslint/eslint, and npm install fails with:

$ npm i
npm ERR! code E404
npm ERR! 404 Not Found - GET https://registry.npmjs.org/@jsr%2fstd__path - Not found
npm ERR! 404
npm ERR! 404  '@jsr/std__path@^1.0.2' is not in this registry.
npm ERR! 404
npm ERR! 404 Note that you can also install from a
npm ERR! 404 tarball, folder, http url, or git url.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we can expect users to change their npm settings so that they can download packages from JSR. We could distribute @std/path in a separate directory inside config-array and import it from there. Or we could use the URL of the tarball from JSR to specify the dependecy in package.json:

-    "@jsr/std__path": "^1.0.2",
+    "@jsr/std__path": "https://npm.jsr.io/~/11/@jsr/std__path/1.0.2.tgz",

Either way, we will lose the magic of the caret ^ to pick newer versions automatically. I checked the JSR docs at https://jsr.io/docs/using-packages but I didn't find any recommendations on how to use their packages with npm apart from registering JSR in .npmrc.

Copy link
Member

Choose a reason for hiding this comment

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

Can one of you open an issue on JSR for this? I can't imagine they didn't encounter this before, but before we spin our wheels, it would be nice to verify if we're doing things the right way.

Copy link
Member Author

Choose a reason for hiding this comment

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

I opened a discussion here: jsr-io/jsr#701

10 changes: 5 additions & 5 deletions packages/config-array/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export default [

In this example, the array contains both config objects and a config array. When a config array is normalized (see details below), it is flattened so only config objects remain. However, the order of evaluation remains the same.

If the `files` array contains a function, then that function is called with the absolute path of the file and is expected to return `true` if there is a match and `false` if not. (The `ignores` array can also contain functions.)
If the `files` array contains a function, then that function is called with the path of the file as it was passed in. The function is expected to return `true` if there is a match and `false` if not. (The `ignores` array can also contain functions.)

If the `files` array contains an item that is an array of strings and functions, then all patterns must match in order for the config to match. In the preceding examples, both `*.test.*` and `*.js` must match in order for the config object to be used.

Expand Down Expand Up @@ -273,7 +273,7 @@ await configs.normalizeSync({
To get the config for a file, use the `getConfig()` method on a normalized config array and pass in the filename to get a config for:

```js
// pass in absolute filename
// pass in filename
const fileConfig = configs.getConfig(
path.resolve(process.cwd(), "package.json"),
);
Expand All @@ -283,14 +283,14 @@ The config array always returns an object, even if there are no configs matching

A few things to keep in mind:

- You must pass in the absolute filename to get a config for.
- If a filename is not an absolute path, it will be resolved relative to the base path directory.
- The returned config object never has `files`, `ignores`, or `name` properties; the only properties on the object will be the other configuration options specified.
- The config array caches configs, so subsequent calls to `getConfig()` with the same filename will return in a fast lookup rather than another calculation.
- A config will only be generated if the filename matches an entry in a `files` key. A config will not be generated without matching a `files` key (configs without a `files` key are only applied when another config with a `files` key is applied; configs without `files` are never applied on their own). Any config with a `files` key entry that is `*` or ends with `/**` or `/*` will only be applied if another entry in the same `files` key matches or another config matches.

## Determining Ignored Paths

You can determine if a file is ignored by using the `isFileIgnored()` method and passing in the absolute path of any file, as in this example:
You can determine if a file is ignored by using the `isFileIgnored()` method and passing in the path of any file, as in this example:

```js
const ignored = configs.isFileIgnored("/foo/bar/baz.txt");
Expand All @@ -304,7 +304,7 @@ A file is considered ignored if any of the following is true:
- **If it matches an entry in `files` and also in `ignores`.** For example, if `**/*.js` is in `files` and `**/a.js` is in `ignores`, then `foo/a.js` and `foo/baz/a.js` are considered ignored.
- **The file is outside the `basePath`.** If the `basePath` is `/usr/me`, then `/foo/a.js` is considered ignored.

For directories, use the `isDirectoryIgnored()` method and pass in the absolute path of any directory, as in this example:
For directories, use the `isDirectoryIgnored()` method and pass in the path of any directory, as in this example:

```js
const ignored = configs.isDirectoryIgnored("/foo/bar/");
Expand Down
26 changes: 26 additions & 0 deletions packages/config-array/fix-std__path-imports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Replace import specifiers in "dist" modules to use the bundled versions of "@jsr/std__path".
*
* In "dist/cjs/index.cjs":
* - '@jsr/std__path/posix' → './std__path/posix.cjs'
* - '@jsr/std__path/windows' → './std__path/windows.cjs'
*
* In "dist/esm/index.js":
* - '@jsr/std__path/posix' → './std__path/posix.js'
* - '@jsr/std__path/windows' → './std__path/windows.js'
*/

import { readFile, writeFile } from "node:fs/promises";

async function replaceInFile(file, search, replacement) {
let text = await readFile(file, "utf-8");
text = text.replace(search, replacement);
await writeFile(file, text);
}

const SEARCH_REGEXP = /'@jsr\/std__path\/(.+?)'/gu;

await Promise.all([
replaceInFile("dist/cjs/index.cjs", SEARCH_REGEXP, "'./std__path/$1.cjs'"),
replaceInFile("dist/esm/index.js", SEARCH_REGEXP, "'./std__path/$1.js'"),
]);
2 changes: 2 additions & 0 deletions packages/config-array/jsr.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
"dist/esm/index.d.ts",
"dist/esm/types.ts",
"dist/esm/types.d.ts",
"dist/esm/std__path/posix.js",
"dist/esm/std__path/windows.js",
"README.md",
"jsr.json",
"LICENSE"
Expand Down
4 changes: 3 additions & 1 deletion packages/config-array/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
"scripts": {
"build:dedupe-types": "node ../../tools/dedupe-types.js dist/cjs/index.cjs dist/esm/index.js",
"build:cts": "node -e \"fs.copyFileSync('dist/esm/index.d.ts', 'dist/cjs/index.d.cts')\"",
"build": "rollup -c && npm run build:dedupe-types && tsc -p tsconfig.esm.json && npm run build:cts",
"build:std__path": "rollup -c rollup.std__path-config.js && node fix-std__path-imports",
"build": "rollup -c && npm run build:dedupe-types && tsc -p tsconfig.esm.json && npm run build:cts && npm run build:std__path",
"test:jsr": "npx jsr@latest publish --dry-run",
"pretest": "npm run build",
"test": "mocha tests/",
Expand All @@ -51,6 +52,7 @@
"minimatch": "^3.1.2"
},
"devDependencies": {
"@jsr/std__path": "^1.0.4",
"@types/minimatch": "^3.0.5",
"c8": "^9.1.0",
"mocha": "^10.4.0",
Expand Down
32 changes: 32 additions & 0 deletions packages/config-array/rollup.std__path-config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { createRequire } from "node:module";

const { resolve } = createRequire(import.meta.url);

export default [
{
input: resolve("@jsr/std__path/posix"),
output: [
{
file: "./dist/cjs/std__path/posix.cjs",
format: "cjs",
},
{
file: "./dist/esm/std__path/posix.js",
format: "esm",
},
],
},
{
input: resolve("@jsr/std__path/windows"),
output: [
{
file: "./dist/cjs/std__path/windows.cjs",
format: "cjs",
},
{
file: "./dist/esm/std__path/windows.js",
format: "esm",
},
],
},
];
Loading