Skip to content

update development environment guide #7612

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

Open
zoran995 wants to merge 2 commits into
base: main
Choose a base branch
from
+46 −22
Open

update development environment guide #7612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
804e90c
update development environment guide
zoran995 Apr 25, 2025
0fe0adb
add cesium build step to guide
zoran995 May 2, 2025
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
2 changes: 2 additions & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

#### next release (8.9.4)

- Update development environment guide
- Update guide for using the custom version of cesium
- [The next improvement]

#### 8.9.3 - 2025-04-24
Expand Down
18 changes: 2 additions & 16 deletions doc/contributing/development-environment.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,14 @@ What if you need to make changes to [TerriaJS](https://github.com/TerriaJS/terri

In the process described in [Cloning and Building](../customizing/cloning-and-building.md), the [TerriaJS package](https://www.npmjs.com/package/terriajs) is installed to the `node_modules` directory by `yarn install`. Please do not edit TerriaJS directly in the `node_modules` directory, because changes will be clobbered the next time you run `yarn install`.

Instead, we want to clone TerriaJS from its [GitHub repo](https://github.com/TerriaJS/terriajs) and use that in our TerriaMap build. Traditionally, `npm link` is the way to do this. However, we do not recommend use of `npm link` because it frequently leads to multiple copies of some libraries being installed, which in turn leads to all sorts of frustrating build problems. Instead, we recommend [yarn](https://yarnpkg.com) and its [workspaces](https://yarnpkg.com/lang/en/docs/workspaces/) feature. `yarn` workspaces let us safely clone a git repo into the `packages` directory and wire it into any other packages that use it.
Instead, we want to clone TerriaJS from its [GitHub repo](https://github.com/TerriaJS/terriajs) and use that in our TerriaMap build. Traditionally, `npm link` is the way to do this. However, we do not recommend use of `npm link` because it frequently leads to multiple copies of some libraries being installed, which in turn leads to all sorts of frustrating build problems. Instead, we recommend and use [yarn](https://yarnpkg.com) and its [workspaces](https://yarnpkg.com/lang/en/docs/workspaces/) feature. `yarn` workspaces let us safely clone a git repo into the `packages` directory and wire it into any other packages that use it.
Copy link
Copy Markdown
Contributor

@pjonsson pjonsson May 7, 2025

Choose a reason for hiding this comment

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

I'm guessing this paragraph reflects the state of things when it was written, but of the projects I've come across from looking at TerriaMap it's quite common of them to use yarn or pnpm so I would guess the average JavaScript developer are already somewhat aware of these tools existence. Can the (old?) npm link-stuff be removed, and the first sentence in this section start with:

To [something] we use yarn workspaces which lets us safely clone a git repository into the packages directory and wire it into any other packages that use it

?


First, install `yarn` globally:
To use `yarn` workspaces you need to have `yarn` installed globally, and you can install it by running

```
npm install -g yarn
```

If you already have yarn installed, make sure it is at least v1.0 (we recommend using the latest version). Older versions do not support the workspace feature.

Then, enable workspaces by editing the TerriaMap `package.json` file, adding these lines to the top, just after the opening `{`:

```json
"private": true,
"workspaces": {
"packages": ["packages/*"],
"nohoist": ["**/husky"]
},
```

Do _not_ commit this change.

Now, you can clone any package (such as `terriajs` or `terriajs-cesium`) into the `packages` directory:
Copy link
Copy Markdown
Collaborator

@niktayv niktayv May 1, 2025

Choose a reason for hiding this comment

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

Would it be more helpful to keep a mention of the "workspaces" property here?

Copy link
Copy Markdown
Collaborator Author

@zoran995 zoran995 May 2, 2025

Choose a reason for hiding this comment

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

Not sure what to add here, we already mentioned yarn workspaces and the user should get familiar with what it is to be able to work with it effectively


```
Copy link
Copy Markdown
Contributor

@pjonsson pjonsson May 7, 2025

Choose a reason for hiding this comment

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

Comment for next line:

For discoverability: can a packages directory containing a README.md be added to the TerriaMap repository? The directory is already in TerriaMap's .gitignore so people will not mistakenly git add more things there. The README can just contain:

This is the directory for yarn workspaces used during development. See [link to this documentation] for how to configure workspaces for TerriaMap.

Expand Down
48 changes: 42 additions & 6 deletions doc/contributing/using-a-custom-version-of-cesium.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,58 @@

What if you need to make changes to [Cesium](https://github.com/AnalyticalGraphicsInc/cesium) while working on TerriaJS?
Copy link
Copy Markdown
Contributor

@pjonsson pjonsson May 7, 2025

Choose a reason for hiding this comment

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

Should this link be updated to point to the Cesium in the CesiumGS project?


TerriaJS is using a fork of Cesium monorepo located at https://github.com/terriajs/cesium, and similar to the upstream repo it consists of two packages:
Copy link
Copy Markdown
Contributor

@pjonsson pjonsson May 7, 2025

Choose a reason for hiding this comment

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

"of Cesium monorepo" => "of the Cesium monorepo"


- [terriajs-cesium](https://www.npmjs.com/package/terriajs-cesium) corresponds to the upstream [@cesium/engine](https://www.npmjs.com/package/@cesium/engine/)
- [terriajs-cesium-widgets](https://www.npmjs.com/package/terriajs-cesium-widgets) corresponds to the upstream [@cesium/widgets](https://www.npmjs.com/package/@cesium/widgets/)

The process of using a custom version of Cesium is much the same as using a custom version of TerriaJS. See the [Development Environment](development-environment.md#building-a-terriamap-against-a-modified-terriajs) for information on setting up and using `yarn`. To clone Cesium, do:

```
```sh
cd packages
git clone -b terriajs https://github.com/TerriaJS/cesium.git
cd ..
git clone https://github.com/TerriaJS/cesium.git terriajs-cesium
```

It is important that you use the `terriajs` branch of [TerriaJS/cesium](https://github.com/TerriaJS/cesium) because it contains important changes to Cesium that are necessary for it to work with TerriaJS. If you need to use a different branch of Cesium, you will need to merge that branch with the changes in the `terriajs` branch.
After cloning the repository, navigate to the `terriajs-cesium` directory and run the following commands to build the Cesium packages and generate TypeScript definitions:

And then run:
```sh
npm run release
Copy link
Copy Markdown
Contributor

@pjonsson pjonsson May 7, 2025

Choose a reason for hiding this comment

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

If you add a cd terriajs-cesium on the first line in this block, it's harder for a tunnel-visioning user to miss the instruction in the text.


# OR

npm install
npm run build
npm run build-ts
```

Make sure that the `terriamap/package.json` workspaces list contains the following packages:
Copy link
Copy Markdown
Contributor

@pjonsson pjonsson May 7, 2025

Choose a reason for hiding this comment

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

Can this paragraph be removed after your TerriaMap-PR is merged?


- "packages/terriajs-cesium/packages/engine",
- "packages/terriajs-cesium/packages/widgets"

Check the versions of `terriajs-cesium` and `terriajs-cesium-widgets` in `terriamap/package.json` and `terriamap/packages/terriajs/package.json` and make sure they are the same so yarn can properly create symlinks.
Comment on lines +29 to +34
Copy link
Copy Markdown
Collaborator

@niktayv niktayv May 1, 2025

Choose a reason for hiding this comment

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

This is very illuminating and I hoped that it would resolve my issues, but despite all my efforts, such as double-checking all the versions and package names alignment in the package.json files, running yarn install runs into an error:

$ yarn install
yarn install v1.18.0
[1/5] 🔍  Validating package.json...
[2/5] 🔍  Resolving packages...
[3/5] 🚚  Fetching packages...
[...]
[5/5] 🔨  Building fresh packages...
error /Users/yuri.vyatkin/Terria/TerriaMap/node_modules/terriajs: Command failed.
Exit code: 1
Command: gulp post-npm-install
Arguments: 
Directory: /Users/yuri.vyatkin/Terria/TerriaMap/node_modules/terriajs
Output:
[14:51:56] Using gulpfile ~/Terria/TerriaMap/packages/terriajs/gulpfile.js
[14:51:56] Starting 'post-npm-install'...
[14:51:56] Starting 'copy-cesium-assets'...
[14:51:56] Starting 'copy-cesium-source-assets'...
[14:51:56] Finished 'copy-cesium-source-assets' after 68 ms
[14:51:56] Starting 'copy-cesium-workers'...
[14:51:56] 'copy-cesium-workers' errored after 1.31 ms
[14:51:56] Error: ENOENT: no such file or directory, scandir '/Users/yuri.vyatkin/Terria/TerriaMap/packages/terriajs-cesium/packages/engine/Build/Workers'
[14:51:56] 'copy-cesium-assets' errored after 70 ms
[14:51:56] 'post-npm-install' errored after 71 ms
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
$

I tried to delete node_modules in all the repos, and tried again, ending up with the same error.
Am I missing something?

Copy link
Copy Markdown
Collaborator

@niktayv niktayv May 1, 2025

Choose a reason for hiding this comment

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

Nanda @na9da clarified that we should run npm run release within packages/terriajs-cesium before running yarn install in TerriaMap.
Now, both yarn install and yarn gulp build work fine in TerriaMap.

Given that I have struggled a lot to grasp that, and I am not alone, perhaps we should explain this in the documentation.

Copy link
Copy Markdown
Collaborator Author

@zoran995 zoran995 May 2, 2025

Choose a reason for hiding this comment

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

added a section above for building cesium packages


Then run:

```
yarn install
```
Comment on lines +36 to 40
Copy link
Copy Markdown
Collaborator

@na9da na9da May 1, 2025

Choose a reason for hiding this comment

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

@zoran995 does running yarn install before building the cesium repo below give an error for you, because post-install terria tries to copy the cesium workers and other assets which it cannot find?

I think before yarn install from terriamap, we need to do:

cd packages/terriajs-cesium
npm install
npm run release

and then:

yarn install # from terriamap

image

Either that, or fix post-npm-install step in terriajs/gulpfile to not fail when it cannot find the cesium assets.

Copy link
Copy Markdown
Collaborator Author

@zoran995 zoran995 May 2, 2025

Choose a reason for hiding this comment

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

added a section for installing cesium dependencies and building the package


## Making the changes in cesium
Copy link
Copy Markdown
Contributor

@pjonsson pjonsson May 7, 2025

Choose a reason for hiding this comment

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

Just grabbing a random point in the diff: Cesium is a name, so it should have capital C in text/headings. (They call it CesiumJS in their own documentation, so maybe that is the name that should be used here as well.)


Cesium uses npm so you should use it instead of yarn when installing dependencies and running commands inside cesium repo. After making the changes you will need to run:
Copy link
Copy Markdown
Contributor

@pjonsson pjonsson May 7, 2025

Choose a reason for hiding this comment

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

If the text doesn't mention yarn, it reduces the risk that a user might skim the text quickly and remember the wrong words:

Cesium uses npm, so use npm when installing dependencies and running commands inside the Cesium repository. After making changes you will need to run:


```sh
npm run release

# OR

npm run build
npm run build-ts
```

to build the code and generate Typescript types definitions required for Terria to work correctly. For more details on building the cesium and coding guidelines consult the [cesium documentation](https://github.com/CesiumGS/cesium/tree/main/Documentation)
Copy link
Copy Markdown
Contributor

@pjonsson pjonsson May 7, 2025

Choose a reason for hiding this comment

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

"Typescript types definitions" => "TypeScript type definitions"
"on building the cesium" => "on building Cesium"


## Committing modifications

If you make changes to Cesium and TerriaJS together, here's the process for getting them to production.
Expand All @@ -38,4 +74,4 @@ Replace `branchName` with the name of the Cesium branch you want to use. You may

Once your Cesium pull request has been merged and a new version of the `terriajs-cesium` npm module has been published, please remember to update `package.json` to point to an official `terriajs-cesium` version instead of a branch in a GitHub repo.

The `package.json` in the `master` branch should always point to official releases of `terriajs-cesium` on npm, NOT GitHub branches.
The `package.json` in the `main` branch should always point to official releases of `terriajs-cesium` on npm, NOT GitHub branches.
Loading