-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Sandcastle Reborn #12574
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
Sandcastle Reborn #12574
Changes from 8 commits
eb55c7e
793ba88
8a73405
c05ed50
4a2082a
1d8cd04
1522aa7
f775608
c3e3f11
db51183
414fc15
463c6b3
5c02885
93955b7
f714fa2
ebafd4d
ab625d8
f574e97
fc38e02
8fa32a4
2ac1ed3
7ecb6f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
/node_modules | ||
packages/sandcastle/node_modules | ||
/ThirdParty | ||
/Tools/** | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ | |
!**/*.html | ||
!**/*.md | ||
!**/*.ts | ||
!**/*.tsx | ||
|
||
# Re-ignore a few things caught above | ||
|
||
|
@@ -33,6 +34,9 @@ packages/widgets/Build/** | |
packages/widgets/index.js | ||
packages/widgets/Source/ThirdParty/** | ||
|
||
packages/sandcastle/node_modules/** | ||
Apps/Sandcastle2/** | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we signal that this is built output by putting it in a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possibly but potentially not very easily given how files are accessed, specifically the sample data. I'd like to leave it here for now to mimic the existing sandcastle as much as possible. |
||
|
||
Specs/jasmine/** | ||
|
||
Apps/Sandcastle/ThirdParty | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,9 @@ | ||
import globals from "globals"; | ||
import html from "eslint-plugin-html"; | ||
import configCesium from "@cesium/eslint-config"; | ||
import reactHooks from "eslint-plugin-react-hooks"; | ||
import reactRefresh from "eslint-plugin-react-refresh"; | ||
import tseslint from "typescript-eslint"; | ||
|
||
export default [ | ||
{ | ||
|
@@ -15,6 +18,8 @@ export default [ | |
"Apps/HelloWorld.html", | ||
"Apps/Sandcastle/jsHintOptions.js", | ||
"Apps/Sandcastle/gallery/gallery-index.js", | ||
"Apps/Sandcastle2/", | ||
"packages/sandcastle/public/", | ||
"packages/engine/Source/Scene/GltfPipeline/**/*", | ||
"packages/engine/Source/Shaders/**/*", | ||
"Specs/jasmine/*", | ||
|
@@ -74,6 +79,31 @@ export default [ | |
sourceType: "module", | ||
}, | ||
}, | ||
...[...tseslint.configs.recommended].map((config) => ({ | ||
// This is needed to restrict to a specific path unless using the tseslint.config function | ||
// https://typescript-eslint.io/packages/typescript-eslint#config | ||
...config, | ||
files: ["packages/sandcastle/**/*.{ts,tsx}"], | ||
})), | ||
{ | ||
// This config came from the vite project generation | ||
files: ["packages/sandcastle/**/*.{ts,tsx}"], | ||
languageOptions: { | ||
ecmaVersion: 2020, | ||
globals: globals.browser, | ||
}, | ||
plugins: { | ||
"react-hooks": reactHooks, | ||
"react-refresh": reactRefresh, | ||
}, | ||
rules: { | ||
...reactHooks.configs.recommended.rules, | ||
"react-refresh/only-export-components": [ | ||
"warn", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be "warn" or "error"? Right now, warnings will not be indicating when running our There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possibly. This was a direct copy out of the generated file from the vite setup just to make it work with the flat eslint config. I honestly didn't really look at what was set or not set yet, completely open to review if you want to take a stab at changing this and extracting like you mention in the other comment. |
||
{ allowConstantExport: true }, | ||
], | ||
}, | ||
Comment on lines
+82
to
+105
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder... To help isolate things a bit more, can we defined the following config in a standalone file and just import it here? I may look into this a bit. No need to hold up the PR. |
||
}, | ||
{ | ||
files: ["Specs/**/*", "packages/**/Specs/**/*"], | ||
languageOptions: { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
# Logs | ||
logs | ||
*.log | ||
npm-debug.log* | ||
yarn-debug.log* | ||
yarn-error.log* | ||
pnpm-debug.log* | ||
lerna-debug.log* | ||
|
||
node_modules | ||
dist | ||
dist-ssr | ||
*.local | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
# CesiumJS Sandcastle | ||
|
||
This package is the application for Sandcastle. | ||
|
||
## Running/Building | ||
|
||
- `npm run dev` from inside this directory to run the development server | ||
- `npm run build` will build this to static files in `/Apps/Sandcastle2` for hosting/access from the normal dev server | ||
- `npm run preview` will run the production build locally. <!--TODO: I'm not sure if we actually need this for our purposes --> | ||
|
||
Linting and style is managed under the project root's scripts. | ||
|
||
## Expanding the ESLint configuration | ||
|
||
<!-- TODO: this section was auto-generated, should figure out if we want these suggestions then remove this --> | ||
|
||
If you are developing a production application, we recommend updating the configuration to enable type-aware lint rules: | ||
|
||
```js | ||
export default tseslint.config({ | ||
extends: [ | ||
// Remove ...tseslint.configs.recommended and replace with this | ||
...tseslint.configs.recommendedTypeChecked, | ||
// Alternatively, use this for stricter rules | ||
...tseslint.configs.strictTypeChecked, | ||
// Optionally, add this for stylistic rules | ||
...tseslint.configs.stylisticTypeChecked, | ||
], | ||
languageOptions: { | ||
// other options... | ||
parserOptions: { | ||
project: ["./tsconfig.node.json", "./tsconfig.app.json"], | ||
tsconfigRootDir: import.meta.dirname, | ||
}, | ||
}, | ||
}); | ||
``` | ||
|
||
You can also install [eslint-plugin-react-x](https://github.com/Rel1cx/eslint-react/tree/main/packages/plugins/eslint-plugin-react-x) and [eslint-plugin-react-dom](https://github.com/Rel1cx/eslint-react/tree/main/packages/plugins/eslint-plugin-react-dom) for React-specific lint rules: | ||
|
||
```js | ||
// eslint.config.js | ||
import reactX from "eslint-plugin-react-x"; | ||
import reactDom from "eslint-plugin-react-dom"; | ||
|
||
export default tseslint.config({ | ||
plugins: { | ||
// Add the react-x and react-dom plugins | ||
"react-x": reactX, | ||
"react-dom": reactDom, | ||
}, | ||
rules: { | ||
// other rules... | ||
// Enable its recommended typescript rules | ||
...reactX.configs["recommended-typescript"].rules, | ||
...reactDom.configs.recommended.rules, | ||
}, | ||
}); | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
<!doctype html> | ||
<html lang="en"> | ||
<head> | ||
<meta charset="UTF-8" /> | ||
<link rel="icon" type="image/svg+xml" href="/favicon.ico" /> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
<title>Sandcastle Reborn</title> | ||
<style> | ||
/* Load fonts for itwin-ui */ | ||
@font-face { | ||
font-family: InterVariable; | ||
font-style: normal; | ||
font-weight: 100 900; | ||
font-display: swap; | ||
src: url("/fonts/InterVariable.woff2") format("woff2"); | ||
} | ||
|
||
@font-face { | ||
font-family: InterVariable; | ||
font-style: italic; | ||
font-weight: 100 900; | ||
font-display: swap; | ||
src: url("/fonts/InterVariable-Italic.woff2") format("woff2"); | ||
} | ||
</style> | ||
</head> | ||
<body> | ||
<div id="app-container"></div> | ||
<script type="module" src="/src/main.tsx"></script> | ||
</body> | ||
</html> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
{ | ||
"name": "@cesium/sandcastle", | ||
"private": true, | ||
"version": "0.0.1", | ||
"type": "module", | ||
"scripts": { | ||
"dev": "vite", | ||
"build": "tsc -b && vite build", | ||
"preview": "vite preview" | ||
}, | ||
"dependencies": { | ||
"@itwin/itwinui-react": "^5.0.0-alpha.14", | ||
"@monaco-editor/react": "^4.7.0", | ||
"monaco-editor": "^0.52.2", | ||
"pako": "^2.1.0", | ||
"react": "^19.0.0", | ||
"react-dom": "^19.0.0" | ||
}, | ||
"devDependencies": { | ||
"@types/pako": "^2.0.3", | ||
"@types/react": "^19.0.10", | ||
"@types/react-dom": "^19.0.4", | ||
"@vitejs/plugin-react": "^4.3.4", | ||
"globals": "^15.15.0", | ||
"typescript": "~5.7.2", | ||
"vite": "^6.2.0", | ||
"vite-plugin-static-copy": "^2.3.1" | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would a blanket
**/node_modules/**
rule work?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically yes but I think we specifically want it to be obvious when dependencies are messed up and it creates a
node_modules
inside the other two packages since that's undesired.