Skip to content

Commit

Permalink
Update createPage to support windows paths (#243
Browse files Browse the repository at this point in the history
Paths like `C:\Users\username\Documents\Github\studio-prototype\packages\studio\tests\__mocks__/test.tsx` are not considered absolute by `path.isAbsolute` in the frontend. This is because we are using `path-browserify` (a browser version of nodejs's `path`) which does not have proper windows support. In real nodejs, `path.isAbsolute` will return true for a windows path like the above.
browserify/path-browserify#1

We could look for an alternative but I'm not sure if that will be necessary. I think I'll have a better picture of that when I'm further in with getting the unit tests compatibility.

For the createPage action, the path.isAbsolute check is unnecessary since we do a `filepath.startsWith(pagesPath)` check as well, and `pagesPath` is always absolute. We may want to support a relative pagesPath at which point we may need to add additional validation, but it would probably make more sense for that validation to go into the backend.

J=SLAP-2791
TEST=auto

tests for createPage.ts and AddPageButton are now passing
  • Loading branch information
oshi97 authored Jun 27, 2023
1 parent 49ceda8 commit f3ae998
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion packages/studio/src/store/StudioActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ export default class StudioActions {
}
const pagesPath = this.getStudioConfig().paths.pages;
const filepath = path.join(pagesPath, pageName + ".tsx");
if (!path.isAbsolute(filepath) || !filepath.startsWith(pagesPath)) {
if (!filepath.startsWith(pagesPath)) {
throw new Error(`Error adding page: pageName is invalid: ${pageName}`);
}
const errorChars = pageName.match(/[\\/?%*:|"<>]/g);
Expand Down

0 comments on commit f3ae998

Please sign in to comment.