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

feat(api): refactor sandpack provider #342

Merged
merged 20 commits into from
Feb 17, 2022
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
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
36 changes: 18 additions & 18 deletions cypress/snapshots.js

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"lint": "eslint '**/*.ts?(x)' --fix",
"format:check": "prettier --check '**/*.{ts,tsx}'",
"format": "prettier --write '**/*.{ts,tsx}'",
"test": "jest sandpack-react",
"test": "jest sandpack-react sandpack-client",
"cy:open": "cypress open",
"cy:run": "cypress run",
"build": "yarn workspace @codesandbox/sandpack-client run build && yarn workspace @codesandbox/sandpack-react run build",
Expand All @@ -33,6 +33,7 @@
"@babel/preset-typescript": "^7.16.5",
"@cypress/snapshot": "^2.1.7",
"@octokit/rest": "^18.12.0",
"@types/jest": "^27.4.0",
"@typescript-eslint/eslint-plugin": "^4.0.0",
"@typescript-eslint/parser": "^4.0.0",
"babel-eslint": "^10.0.0",
Expand Down
11 changes: 8 additions & 3 deletions sandpack-client/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ export class SandpackClient {
const element = document.querySelector(selector);

if (!element) {
throw new Error(`No element found for selector '${selector}'`);
throw new Error(
`[sandpack-client]: the element '${selector}' was not found`
);
}

this.element = element;
Expand Down Expand Up @@ -254,7 +256,8 @@ export class SandpackClient {
packageJSON = JSON.parse(files["/package.json"].code);
} catch (e) {
console.error(
"Could not parse package.json file: " + (e as Error).message
"[sandpack-client]: could not parse package.json file: " +
(e as Error).message
);
}

Expand Down Expand Up @@ -375,7 +378,9 @@ export class SandpackClient {

if (!this.element.parentNode) {
// This should never happen
throw new Error("Given element does not have a parent.");
throw new Error(
`[sandpack-client]: the given iframe does not have a parent.`
);
}

this.element.parentNode.replaceChild(this.iframe, this.element);
Expand Down
80 changes: 80 additions & 0 deletions sandpack-client/src/utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { addPackageJSONIfNeeded } from "./utils";

const files = {
"/package.json": {
code: `{
"name": "custom-package",
"main": "old-entry.js",
"dependencies": { "baz": "*" },
"devDependencies": { "baz": "*" }
}`,
},
};

describe(addPackageJSONIfNeeded, () => {
test("it merges the package.json - dependencies", () => {
const output = addPackageJSONIfNeeded(files, { foo: "*" });

expect(JSON.parse(output["/package.json"].code).dependencies).toEqual({
baz: "*",
foo: "*",
});
});

test("it merges the package.json - dev-dependencies", () => {
const output = addPackageJSONIfNeeded(files, undefined, { foo: "*" });

expect(JSON.parse(output["/package.json"].code).devDependencies).toEqual({
baz: "*",
foo: "*",
});
});

test("it merges the package.json - entry", () => {
const output = addPackageJSONIfNeeded(
files,
undefined,
undefined,
"new-entry.js"
);

expect(JSON.parse(output["/package.json"].code).main).toEqual(
"new-entry.js"
);
});

test("it returns an error when there is not dependencies at all", () => {
try {
addPackageJSONIfNeeded({ "/package.json": { code: `{}` } });

// eslint-disable-next-line @typescript-eslint/no-explicit-any
} catch (err: any) {
expect(err.message).toBe(
'[sandpack-client]: "dependencies" was not specified - provide either a package.json or a "dependencies" value'
);
}
});

test("it supports package.json without leading slash", () => {
const output = addPackageJSONIfNeeded(
{
"package.json": {
code: `{
"main": "old-entry.js",
"dependencies": { "baz": "*" },
"devDependencies": { "baz": "*" }
}`,
},
},
{ foo: "*" },
{ foo: "*" },
"new-entry.ts"
);

expect(JSON.parse(output["package.json"].code)).toEqual({
main: "new-entry.ts",
dependencies: { baz: "*", foo: "*" },
devDependencies: { baz: "*", foo: "*" },
});
});
});
62 changes: 51 additions & 11 deletions sandpack-client/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import type {
ErrorStackFrame,
} from "./types";

const DEPENDENCY_ERROR_MESSAGE = `[sandpack-client]: "dependencies" was not specified - provide either a package.json or a "dependencies" value`;
const ENTRY_ERROR_MESSAGE = `[sandpack-client]: "entry" was not specified - provide either a package.json with the "main" field or na "entry" value`;

export function createPackageJSON(
dependencies: Dependencies = {},
devDependencies: Dependencies = {},
Expand All @@ -31,22 +34,59 @@ export function addPackageJSONIfNeeded(
): SandpackBundlerFiles {
const newFiles = { ...files };

if (!newFiles["/package.json"]) {
if (!dependencies) {
throw new Error(
"No dependencies specified, please specify either a package.json or dependencies."
);
}
const getPackageJsonFile = (): string | undefined => {
if (newFiles["/package.json"]) return "/package.json";
if (newFiles["package.json"]) return "package.json";
Copy link
Member

Choose a reason for hiding this comment

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

Should we normalise these paths at some point? Just a note for potential future improvements

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, indeed we need to address it. I noted this problem when I was trying to implement a new feature, which fetches sandboxes from CodeSandbox, and I faced a ton of issues (see resolveFile for instance).

Here's the task: #353


if (!entry) {
throw new Error(
"Missing 'entry' parameter. Either specify an entry point, or pass in a package.json with the 'main' field set."
);
}
return undefined;
};
const packageJsonFile = getPackageJsonFile();

/**
* Create a new package json
*/
if (!packageJsonFile) {
if (!dependencies) throw new Error(DEPENDENCY_ERROR_MESSAGE);
if (!entry) throw new Error(ENTRY_ERROR_MESSAGE);

newFiles["/package.json"] = {
code: createPackageJSON(dependencies, devDependencies, entry),
};

return newFiles;
}

/**
* Merge package json with custom setup
*/
if (packageJsonFile) {
const packageJsonContent = JSON.parse(newFiles[packageJsonFile].code);

if (!dependencies && !packageJsonContent.dependencies) {
throw new Error(DEPENDENCY_ERROR_MESSAGE);
}

if (dependencies) {
packageJsonContent.dependencies = {
...(packageJsonContent.dependencies ?? {}),
...(dependencies ?? {}),
};
}

if (devDependencies) {
packageJsonContent.devDependencies = {
...(packageJsonContent.devDependencies ?? {}),
...(devDependencies ?? {}),
};
}

if (entry) {
packageJsonContent.main = entry;
}

newFiles[packageJsonFile] = {
code: JSON.stringify(packageJsonContent, null, 2),
};
}

return newFiles;
Expand Down
2 changes: 1 addition & 1 deletion sandpack-client/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@
"skipLibCheck": true
},
"include": ["./src"],
"exclude": ["node_modules"]
"exclude": ["node_modules", "src/**/*.test.ts"]
}
14 changes: 7 additions & 7 deletions sandpack-react/src/components/CodeEditor/CodeEditor.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ export default {

export const Component: Story<CodeEditorProps> = (args) => (
<SandpackProvider
files={{
"/index.js": {
code: 'const title = "This is a simple code editor"',
},
}}
customSetup={{
entry: "/index.js",
files: {
"/index.js": {
code: 'const title = "This is a simple code editor"',
},
},
}}
>
<SandpackThemeProvider>
Expand Down Expand Up @@ -90,13 +90,13 @@ export const ReadOnly: React.FC = () => {
<>
<p>Read-only by file</p>
<Sandpack
customSetup={{ entry: "/index.tsx", main: "/App.tsx" }}
customSetup={{ entry: "/index.tsx" }}
files={{
"/index.tsx": { code: "", hidden: true },
"/src/App.tsx": { code: "Hello", readOnly: true, active: true },
"/src/components/button.tsx": { code: "World", readOnly: false },
}}
options={{ showTabs: true }}
options={{ showTabs: true, activePath: "/App.tsx" }}
template="react-ts"
/>

Expand Down
16 changes: 8 additions & 8 deletions sandpack-react/src/components/CodeViewer/CodeViewer.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ export const Component: Story<CodeViewerProps> = (args) => (
<SandpackProvider
customSetup={{
entry: "/index.js",
files: {
"/index.js": {
code: `const people = [{
}}
files={{
"/index.js": {
code: `const people = [{
id: 0,
name: 'Creola Katherine Johnson',
profession: 'mathematician',
Expand All @@ -34,7 +35,6 @@ export default function List() {
);
return <ul>{listItems}</ul>;
}`,
},
},
}}
>
Expand Down Expand Up @@ -65,9 +65,10 @@ export const Decorators: React.FC = () => {
<SandpackProvider
customSetup={{
entry: "/index.js",
files: {
"/index.js": {
code: `const people = [{
}}
files={{
"/index.js": {
code: `const people = [{
id: 0,
name: 'Creola Katherine Johnson',
profession: 'mathematician',
Expand All @@ -84,7 +85,6 @@ export default function List() {
);
return <ul>{listItems}</ul>;
}`,
},
},
}}
>
Expand Down
20 changes: 10 additions & 10 deletions sandpack-react/src/components/FileExplorer/FileExplorer.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ export const Component: React.FC = () => (
<SandpackProvider
customSetup={{
entry: "/index.tsx",
files: {
"/index.tsx": "",
"/src/app.tsx": "",
"/src/components/button.tsx": "",
},
}}
files={{
"/index.tsx": "",
"/src/app.tsx": "",
"/src/components/button.tsx": "",
}}
>
<SandpackLayout>
Expand All @@ -30,11 +30,11 @@ export const Component: React.FC = () => (
<SandpackProvider
customSetup={{
entry: "/index.tsx",
files: {
"/index.tsx": "",
"/src/app.tsx": "",
"/src/components/button.tsx": "",
},
}}
files={{
"/index.tsx": "",
"/src/app.tsx": "",
"/src/components/button.tsx": "",
}}
>
<SandpackLayout theme="night-owl">
Expand Down
31 changes: 15 additions & 16 deletions sandpack-react/src/components/FileTabs/FileTabs.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as React from "react";

import { Sandpack } from "../../";
import { SandpackLayout } from "../../common/Layout";
import { SandpackProvider } from "../../contexts/sandpackContext";
import { SandpackCodeViewer } from "../CodeViewer";
Expand All @@ -15,11 +14,11 @@ export const Component: React.FC = () => (
<SandpackProvider
customSetup={{
entry: "/index.tsx",
files: {
"/index.tsx": "",
"/src/app.tsx": { code: "", active: true },
"/src/components/button.tsx": "",
},
}}
files={{
"/index.tsx": "",
"/src/app.tsx": { code: "", active: true },
"/src/components/button.tsx": "",
}}
>
<SandpackLayout>
Expand All @@ -32,11 +31,11 @@ export const WithClosableTabs: React.FC = () => (
<SandpackProvider
customSetup={{
entry: "/index.tsx",
files: {
"/index.tsx": { code: "", hidden: true },
"/src/app.tsx": "Hello",
"/src/components/button.tsx": "World",
},
}}
files={{
"/index.tsx": { code: "", hidden: true },
"/src/app.tsx": "Hello",
"/src/components/button.tsx": "World",
}}
>
<SandpackLayout>
Expand All @@ -49,11 +48,11 @@ export const WithHiddenFiles: React.FC = () => (
<SandpackProvider
customSetup={{
entry: "/index.tsx",
files: {
"/index.tsx": { code: "", hidden: true },
"/src/app.tsx": "Hello",
"/src/components/button.tsx": "World",
},
}}
files={{
"/index.tsx": { code: "", hidden: true },
"/src/app.tsx": "Hello",
"/src/components/button.tsx": "World",
}}
>
<SandpackLayout>
Expand Down
Loading