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.

Sign up for GitHub

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

Jump to bottom

Windows #3

Merged
merged 13 commits into from
Sep 25, 2023
Merged

Windows #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
fe07092
fix windows related path issues
edivados Sep 9, 2023
db94dbb
Merge branch 'main' into windows
edivados Sep 22, 2023
d7ed40d
more path issues
edivados Sep 22, 2023
51b6d84
Merge branch 'main' into windows
edivados Sep 22, 2023
716c7eb
Merge branch 'main' into windows
edivados Sep 23, 2023
0244151
refactor base file system router globbing
edivados Sep 24, 2023
54f1a15
Merge branch 'main' into windows
edivados Sep 24, 2023
e954b8b
add pathe back to router dev plugins
edivados Sep 24, 2023
c24680c
update pnpm-lock.yaml
edivados Sep 24, 2023
acf955c
remove stuff from other branch
edivados Sep 24, 2023
ad5d6f1
base file system router glob pattern again
edivados Sep 25, 2023
fc52fd2
Merge branch 'main' into windows
edivados Sep 25, 2023
d3b8eed
fix hmr
edivados Sep 25, 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
11 changes: 5 additions & 6 deletions packages/vinxi/lib/build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ import {
} from "h3";
import { createRequire } from "module";
import { build, copyPublicAssets, createNitro } from "nitropack";
import { join } from "path";
import { relative } from "pathe";
import { join, relative } from "pathe";

import { writeFileSync } from "node:fs";

Expand All @@ -34,7 +33,7 @@ const require = createRequire(import.meta.url);
*/
export async function createBuild(app, buildConfig) {
const { existsSync, promises: fsPromises, readFileSync } = await import("fs");
const { join } = await import("path");
const { join } = await import("pathe");
Copy link
Contributor Author

@edivados edivados Sep 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

These imports would fail because of characters being escaped generating paths like C:goodjobwindows

vinxi/packages/vinxi/lib/build.js

Lines 457 to 460 in 505dd51

import middleware from "${join(config.router.root, config.router.middleware)}";
import handler from "${join(config.router.root, config.router.handler)}";
import { eventHandler } from "vinxi/runtime/server";
export default eventHandler({ onRequest: middleware.onRequest, onBeforeResponse: middleware.onBeforeResponse, handler});`;

const { fileURLToPath } = await import("url");
for (const router of app.config.routers) {
if (existsSync(router.outDir)) {
Expand Down Expand Up @@ -102,7 +101,7 @@ export async function createBuild(app, buildConfig) {
? "virtual:#vinxi/handler"
: relative(app.config.root, router.handler)
].file,
).replace(/\\/g, "\\\\");
);

return [
{
Expand Down Expand Up @@ -543,7 +542,7 @@ function handerBuild() {
"Invalid router",
);
const { builtinModules } = await import("module");
const { join } = await import("path");
const { join } = await import("pathe");
const input = await getEntries(inlineConfig.router);
return {
build: {
Expand Down Expand Up @@ -582,7 +581,7 @@ function browserBuild() {
inlineConfig.router && inlineConfig.router.mode !== "static",
"Invalid router",
);
const { join } = await import("path");
const { join } = await import("pathe");
return {
build: {
rollupOptions: {
Expand Down
13 changes: 5 additions & 8 deletions packages/vinxi/lib/fs-router.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import fg from "fast-glob";
import fs from "fs";
import micromatch from "micromatch";
import os from "os";
import { join } from "path";
import { posix } from "path";
import { pathToRegexp } from "path-to-regexp";

export { pathToRegexp };
Expand Down Expand Up @@ -76,13 +76,10 @@ export class BaseFileSystemRouter extends EventTarget {
}

glob() {
if (os.platform() === "win32") {
return fg.convertPathToPattern(this.config.dir + "//**//*");
} else {
return (
join(this.config.dir, "**/*") + `.{${this.config.extensions.join(",")}}`
);
}
return posix.join(
fg.convertPathToPattern(this.config.dir),
"**/*"
) + `.{${this.config.extensions.join(",")}}`;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/vinxi/lib/manifest/collect-styles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"use strict";

import { isBuiltin } from "node:module";
import { join, resolve } from "node:path";
import { join, resolve } from "pathe";

async function getViteModuleNode(vite, file, ssr) {
if (file.startsWith("node:") || isBuiltin(file)) {
Expand All @@ -19,7 +19,7 @@ async function getViteModuleNode(vite, file, ssr) {

const id = resolvedId.id;

const normalizedPath = resolve(id).replace(/\\/g, "/");
const normalizedPath = resolve(id);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@solid-refresh gets converted to C:/@solid-refresh causing errors


try {
let node = await vite.moduleGraph.getModuleById(normalizedPath);
Expand Down
4 changes: 2 additions & 2 deletions packages/vinxi/lib/manifest/dev-server-manifest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export function createDevManifest(app) {
} else {
return {
output: {
path: absolutePath,
path: join(router.base, "@fs", absolutePath),
Copy link
Owner

Choose a reason for hiding this comment

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

this change will not in some modes: this change will not work, on the server we don't want to use @fs links. those are for the client. this is being used with the server actions setup. I should add a test for that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nksaraf What about LN 175? Should that also be reverted in that case? I did this in two places.

vinxi/packages/vinxi/lib/manifest/dev-server-manifest.js

Line 175 in 2fa85e2

path: join(router.base, "@fs", absolutePath),

},
};
}
Expand Down Expand Up @@ -173,7 +173,7 @@ export function createDevManifest(app) {
];
},
output: {
path: absolutePath,
path: join(router.base, "@fs", absolutePath),
Copy link
Contributor Author

@edivados edivados Sep 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure on this one but seems to work. If I remember it right dynamic import in lazy route would not work.

vinxi/packages/vinxi-solid/lazy-route.jsx

Lines 20 to 22 in 505dd51

const mod = await import(
/* @vite-ignore */ manifest.inputs[component.src].output.path
);

},
};
}
Expand Down
3 changes: 2 additions & 1 deletion packages/vinxi/lib/plugins/fs-watcher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { fileURLToPath } from "node:url";
import { normalize } from "pathe";

/**
*
Expand Down Expand Up @@ -43,7 +44,7 @@ export const fileSystemWatcher = () => {
config.router.internals.routes.addEventListener("reload", () => {
const { moduleGraph } = server;
const mods = moduleGraph.getModulesByFile(
fileURLToPath(new URL("../routes.js", import.meta.url)),
normalize(fileURLToPath(new URL("../routes.js", import.meta.url))),
);
if (mods) {
const seen = new Set();
Expand Down
2 changes: 1 addition & 1 deletion packages/vinxi/lib/plugins/routes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { relative } from "node:path";
import { relative } from "pathe";
import { fileURLToPath } from "node:url";
Copy link
Contributor Author

@edivados edivados Sep 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Manifest lookups in build would fail because of \.

vinxi/packages/vinxi/lib/plugins/routes.js

Line 77 in 505dd51

src: isBuild ? relative(root, buildId) : buildId,


/**
Expand Down
2 changes: 1 addition & 1 deletion packages/vinxi/lib/router-dev-plugins.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { join } from "node:path";
import { join } from "pathe";

import { devEntries } from "./dev-server.js";
import invariant from "./invariant.js";
Expand Down
Loading