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

chore: Peer dependency script experiment #2450

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
18 changes: 18 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,24 @@ If the changes are expected, call this out in your pull request comments.
└── style-dictionary - style dictionary tokens and roles
```

### Using a peer dependency from a branch

Add `scripts/install-peer-dependency.js` as postinstall:
```
"scripts": {
"postinstall": "node ./scripts/install-peer-dependency.js <package-name>:<target-branch>",
}
```

For example:
```
"scripts": {
"postinstall": "node ./scripts/install-peer-dependency.js collection-hooks:new-feature",
}
```

Run `npm install`.

## Code of Conduct

This project has adopted the [Amazon Open Source Code of Conduct](https://aws.github.io/code-of-conduct).
Expand Down
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"files": [],
"scripts": {
"quick-build": "gulp quick-build",
"postinstall": "node ./scripts/install-peer-dependency.js collection-hooks:property-filter-token-groups",
"build": "cross-env NODE_ENV=production gulp build",
"test": "TZ=UTC gulp test",
"test:unit": "TZ=UTC gulp test:unit",
Expand Down Expand Up @@ -158,4 +159,4 @@
"last 3 Edge major versions",
"last 3 Safari major versions"
]
}
}
55 changes: 55 additions & 0 deletions scripts/install-peer-dependency.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
#!/usr/bin/env node
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

const { execSync } = require('child_process');
const path = require('path');
const os = require('os');

const args = process.argv.slice(2);
if (args.length < 1) {
console.error('Usage: install-peer-dependency.js <package-name>:<target-branch>');
process.exit(1);
}
const [packageName, targetBranch] = args[0].split(':');
const targetRepository = `https://github.com/cloudscape-design/${packageName}.git`;
const nodeModulesPackagePath = path.join(__dirname, '..', 'node_modules', '@cloudscape-design', packageName);
const tempDir = path.join(os.tmpdir(), `temp-${packageName}`);

// Clone the repository and checkout the branch
console.log(`Cloning ${packageName}:${targetBranch}...`);
execCommand(`git clone ${targetRepository} ${tempDir}`);
process.chdir(tempDir);
execCommand(`git checkout ${targetBranch}`);

// Install dependencies and build
console.log(`Installing dependencies and building ${packageName}...`);
execCommand('npm install');
execCommand('npm run build');

// Remove existing peer dependency in node_modules
console.log(`Removing existing ${packageName} from node_modules...`);
execCommand(`rm -rf ${nodeModulesPackagePath}`);

// Copy built peer dependency to node_modules
console.log(`Copying built ${targetRepository} to node_modules...`);
execCommand(`mkdir -p ${nodeModulesPackagePath}`);
execCommand(`cp -R ${tempDir}/lib/* ${nodeModulesPackagePath}`);
Copy link
Member Author

Choose a reason for hiding this comment

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

This part is shaky. In collection-hooks we package the distribution to the lib folder entirely. In other packages we might use the root folder or dist. We can either parametrise this line or make all packages better consistent.

Copy link
Member

@michaeldowseza michaeldowseza Jul 10, 2024

Choose a reason for hiding this comment

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

This is not an issue. We standardised everything to package to lib folder 👍🏻

Copy link
Member

Choose a reason for hiding this comment

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

Where it might be an issue is some packages produce multiple artifacts in the lib folder. E.g. test-utils, theming-core, components, board-components, code-view


// Clean up
console.log('Cleaning up...');
execCommand(`rm -rf ${tempDir}`);

console.log(`${packageName} has been successfully installed from branch ${targetBranch}!`);

function execCommand(command, options = {}) {
try {
execSync(command, { stdio: 'inherit', ...options });

Check warning

Code scanning / CodeQL

Shell command built from environment values Medium

This shell command depends on an uncontrolled
absolute path
.
Copy link
Member Author

Choose a reason for hiding this comment

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

Can we dismiss that?

} catch (error) {
console.error(`Error executing command: ${command}`);
console.error(`Error message: ${error.message}`);
console.error(`Stdout: ${error.stdout && error.stdout.toString()}`);
console.error(`Stderr: ${error.stderr && error.stderr.toString()}`);
throw error;
}
}
3 changes: 3 additions & 0 deletions src/property-filter/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
PropertyFilterProperty,
PropertyFilterToken,
} from '@cloudscape-design/collection-hooks';
import { PropertyFilterTokenGroup } from '@cloudscape-design/collection-hooks/cjs/interfaces';
Copy link
Member Author

Choose a reason for hiding this comment

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

These are temporary changes just to prove that the PR indeed fetches code from the collection-hooks branch: cloudscape-design/collection-hooks#74


export interface PropertyFilterProps extends BaseComponentProps, ExpandToViewport, FormFieldControlProps {
/**
Expand Down Expand Up @@ -212,6 +213,7 @@ export interface PropertyFilterProps extends BaseComponentProps, ExpandToViewpor

export namespace PropertyFilterProps {
export type Token = PropertyFilterToken;
export type TokenGroup = PropertyFilterTokenGroup;
export type JoinOperation = PropertyFilterOperation;
export type ComparisonOperator = PropertyFilterOperator;
export type ExtendedOperator<TokenValue> = PropertyFilterOperatorExtended<TokenValue>;
Expand Down Expand Up @@ -304,6 +306,7 @@ export namespace PropertyFilterProps {
// Re-exported namespace interfaces to use module-style imports internally

export type Token = PropertyFilterProps.Token;
export type TokenGroup = PropertyFilterProps.TokenGroup;
export type JoinOperation = PropertyFilterProps.JoinOperation;
export type ComparisonOperator = PropertyFilterProps.ComparisonOperator;
export type ExtendedOperator<TokenValue> = PropertyFilterOperatorExtended<TokenValue>;
Expand Down
Loading