Skip to content

Commit 5908dd0

Browse files
ArtmannArtmann
committed
pr feedback
1 parent bdf0c6b commit 5908dd0

14 files changed

+44
-45
lines changed

‎build/add-js-extensions.mjs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,9 @@
55
import { promises as fs } from 'fs';
66
import path from 'path';
77
import { fileURLToPath } from 'url';
8-
import { dirname } from 'path';
98

109
const __filename = fileURLToPath(import.meta.url);
11-
const __dirname = dirname(__filename);
10+
const __dirname = path.dirname(__filename);
1211

1312
const rootDir = path.join(__dirname, '..');
1413
const srcDir = path.join(rootDir, 'src');

‎build/constants.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4-
import { ExtensionRootDir as _ExtensionRootDir } from './util.js';
5-
6-
export const ExtensionRootDir = _ExtensionRootDir;
4+
export { ExtensionRootDir } from './util.js';
75
export const isWindows = /^win/.test(process.platform);
86
export const isCI = process.env.TF_BUILD !== undefined || process.env.GITHUB_ACTIONS === 'true';

‎build/fix-directory-imports.mjs‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -44,19 +44,15 @@ function fixDirectoryImports(content) {
4444
let changeCount = 0;
4545

4646
for (const dirImport of directoryImports) {
47-
// Match imports like '../platform/logging.js' or './platform/logging.js'
48-
const patterns = [
49-
new RegExp(`(['"])(\\.\\.?\\/.*?\\/)${dirImport}\\.js(['"])`, 'g'),
50-
new RegExp(`(['"])(\\.\\.?\\/)${dirImport}\\.js(['"])`, 'g'),
51-
];
52-
53-
for (const pattern of patterns) {
54-
const newModified = modified.replace(pattern, (match, quote1, pathPrefix, quote2) => {
55-
changeCount++;
56-
return `${quote1}${pathPrefix}${dirImport}/index.js${quote2}`;
57-
});
58-
modified = newModified;
59-
}
47+
// Match imports with any relative path prefix (./, ../, ../../foo/, etc.)
48+
// Captures: quote, prefix (e.g., './', '../../'), directoryImport, '.js', quote
49+
const pattern = new RegExp(`(['"])((?:\\.\\.\\/|\\.\\/)(?:.*\\/)?)${dirImport}\\.js(['"])`, 'g');
50+
51+
const newModified = modified.replace(pattern, (match, quote1, prefix, quote2) => {
52+
changeCount++;
53+
return `${quote1}${prefix}${dirImport}/index.js${quote2}`;
54+
});
55+
modified = newModified;
6056
}
6157

6258
return { content: modified, changed: changeCount > 0, changeCount };

‎build/fix-telemetry-imports.mjs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,14 @@ function fixTelemetryImports(content) {
3838
// Fix: './telemetry/index' -> './telemetry' (top-level telemetry.ts file)
3939
// Fix: '../telemetry/index' -> '../telemetry'
4040
// Fix: '../../telemetry/index' -> '../../telemetry' etc.
41-
const pattern = /(from\s+['"])(\.\.\/?)+telemetry\/index(['"'])/g;
41+
const pattern = /(from\s+['"])((?:\.\.\/?)+)telemetry\/index(['"'])/g;
4242
modified = modified.replace(pattern, (match, before, dots, after) => {
4343
changeCount++;
4444
return `${before}${dots}telemetry${after}`;
4545
});
4646

4747
// Fix the double path: './platform/telemetry/telemetry/index' -> './platform/telemetry'
48-
const doublePath = /(from\s+['"])(\.\.\/?)+platform\/telemetry\/telemetry\/index(['"'])/g;
48+
const doublePath = /(from\s+['"])((?:\.\.\/?)+)platform\/telemetry\/telemetry\/index(['"'])/g;
4949
modified = modified.replace(doublePath, (match, before, dots, after) => {
5050
changeCount++;
5151
return `${before}${dots}platform/telemetry${after}`;

‎build/postDebugWebTest.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,9 @@
44
import fs from 'fs-extra';
55
import path from 'node:path';
66
import { fileURLToPath } from 'node:url';
7-
import { dirname } from 'node:path';
87

98
const __filename = fileURLToPath(import.meta.url);
10-
const __dirname = dirname(__filename);
9+
const __dirname = path.dirname(__filename);
1110

1211
try {
1312
const file = path.join(__dirname, '..', 'temp', 'jupyter.pid');

‎build/preDebugWebTest.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ async function go() {
2222

2323
const tempDir = path.join(__dirname, '..', 'temp');
2424

25-
await fs.ensureDir(tempDir);
25+
fs.ensureDirSync(tempDir);
2626

27-
fs.writeFileSync(path.join(__dirname, '..', 'temp', 'deepnote.pid'), info.server.pid.toString());
27+
fs.writeFileSync(path.join(tempDir, 'deepnote.pid'), info.server.pid.toString());
2828
} else {
2929
console.log('Jupyter server URL provided in env args, no need to start one');
3030
}

‎build/preLaunchWebTest.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Licensed under the MIT License.
33

44
import path from 'node:path';
5-
import jupyterServer from '../out/test/datascience/jupyterServer.node';
5+
import jupyterServer from '../out/test/datascience/jupyterServer.node.js';
66
import fs from 'fs-extra';
77
import { fileURLToPath } from 'node:url';
88
import { dirname } from 'node:path';

‎build/remove-js-extensions.mjs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,9 @@
55
import { promises as fs } from 'fs';
66
import path from 'path';
77
import { fileURLToPath } from 'url';
8-
import { dirname } from 'path';
98

109
const __filename = fileURLToPath(import.meta.url);
11-
const __dirname = dirname(__filename);
10+
const __dirname = path.dirname(__filename);
1211

1312
const rootDir = path.join(__dirname, '..');
1413
const srcDir = path.join(rootDir, 'src');

‎build/util.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,9 @@
44
import fs from 'node:fs';
55
import path from 'node:path';
66
import { fileURLToPath } from 'node:url';
7-
import { dirname } from 'node:path';
87

98
const __filename = fileURLToPath(import.meta.url);
10-
const __dirname = dirname(__filename);
9+
const __dirname = path.dirname(__filename);
1110

1211
export const ExtensionRootDir = path.dirname(__dirname);
1312

‎gulpfile.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ gulp.task('validateTranslationFiles', (done) => {
7070
glob.sync('package.nls.*.json', { sync: true }).forEach((file) => {
7171
// Verify we can open and parse as JSON.
7272
try {
73-
const js = JSON.parse(fs.readFileSync(file));
73+
const js = JSON.parse(fs.readFileSync(file, 'utf-8'));
7474
const result = validator.validate(js, schema);
7575
if (Array.isArray(result.errors) && result.errors.length) {
7676
console.error(result.errors);
@@ -271,17 +271,17 @@ gulp.task('validateTelemetry', async () => {
271271

272272
gulp.task('validatePackageLockJson', async () => {
273273
const fileName = path.join(__dirname, 'package-lock.json');
274-
const oldContents = fs.readFileSync(fileName).toString();
274+
const oldContents = fs.readFileSync(fileName, 'utf-8');
275275
spawnSync('npm', ['install', '--prefer-offline']);
276-
const newContents = fs.readFileSync(fileName).toString();
276+
const newContents = fs.readFileSync(fileName, 'utf-8');
277277
if (oldContents.trim() !== newContents.trim()) {
278278
throw new Error('package-lock.json has changed after running `npm install`');
279279
}
280280
});
281281

282282
gulp.task('verifyUnhandledErrors', async () => {
283283
const fileName = path.join(__dirname, 'unhandledErrors.txt');
284-
const contents = fs.pathExistsSync(fileName) ? fs.readFileSync(fileName, 'utf8') : '';
284+
const contents = fs.pathExistsSync(fileName) ? fs.readFileSync(fileName, 'utf-8') : '';
285285
if (contents.trim().length) {
286286
console.error(contents);
287287
throw new Error('Unhandled errors detected. Please fix them before merging this PR.', contents);

0 commit comments

Comments
 (0)