Skip to content

Commit

Permalink
minor #2326 [DX] Add per-package Yarn scripts (build, watch, test, li…
Browse files Browse the repository at this point in the history
…nt, ...) (Kocal)

This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[DX] Add per-package Yarn scripts (build, watch, test, lint, ...)

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Issues        | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT

<!--
Replace this notice by a description of your feature/bugfix.
This will help reviewers and should be a good start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - For new features, provide some code snippets to help understand usage.
 - Features and deprecations must be submitted against branch main.
 - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
 - Never break backward compatibility (see https://symfony.com/bc).
-->

When working in UX Map, it was very painful to build or watch modifications on `.ts` files, I had investigate how the building process worked, and had to run Rollup like this, which is far from ideal (not documented and not obvious):
```
./node_modules/.bin/rollup -c --environment INPUT_FILE:src/Map/assets/src/abstract_map_controller.ts -w
./node_modules/.bin/rollup -c --environment INPUT_FILE:src/Map/src/Bridge/Google/assets/src/map_controller.ts -w
./node_modules/.bin/rollup -c --environment INPUT_FILE:src/Map/src/Bridge/Leaflet/assets/src/map_controller.ts -w
```

And I also felt sorry for `@rrr63` when he worked on adding Polygons on Map (#2162)...

This PR improves the way developers will work on UX, and makes their lives easier.

**Before**:
- `yarn build` compiled the assets from ALL packages, it was not possible to build packages from only one package (which is useful if you work on a single package)
- no `yarn watch`
- `yarn test` runned tests from ALL packages, like `yarn build`, it was not possible to run tests for only one package

**Now:**
- at the project root, `build`/`test`/`lint`/`format`/`check-lint`/`check-format` scripts will run on all assets **from all packages**. And it will be faster than before, when processing was sequential, but now it's parallelized.
- at a package root (ex:  `src/Map/assets`), `build`/`test`/`lint`/`format`/`check-lint`/`check-format` scripts will run on all assets **from this package only**
- `build` and `watch` scripts handles both TypeScript and CSS files in a single command

This is a first step to what we spoke about with `@smnandre` to write a contribution guide.
It is now much more easier and friendlier to tell a developer to run `yarn watch` inside a package root, instead of telling it to run `./node_modules/.bin/rollup -c --environment INPUT_FILE:src/Map/src/Bridge/Google/assets/src/map_controller.ts -w` (and more, if you have multiple files).

<img width="1210" alt="image" src="https://github.com/user-attachments/assets/f40d4efc-439d-4f83-a0f8-611b1a64b334">

Commits
-------

b8dc5fd Ignore `var` folder in Biome.js
8397b52 [DX] Rework testing process, add `test` script to packages package.json
bc1d63c [DX] Reconfigure lint/format tasks to not run the command for each workspace, since Biome is super fast (also include bin and test JS files)
1873b3e [DX] Rework building process, and add build/watch scripts to packages package.json
5517388 [DX] Add lint/format/check-lint/check-format scripts to packages package.json
  • Loading branch information
Kocal committed Nov 3, 2024
2 parents faaa385 + b8dc5fd commit 57f7314
Show file tree
Hide file tree
Showing 30 changed files with 481 additions and 338 deletions.
43 changes: 0 additions & 43 deletions bin/build_javascript.js

This file was deleted.

128 changes: 128 additions & 0 deletions bin/build_package.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
/**
* This file is used to compile the assets from an UX package.
*/

const { parseArgs } = require('node:util');
const path = require('node:path');
const fs = require('node:fs');
const glob = require('glob');
const rollup = require('rollup');
const CleanCSS = require('clean-css');
const { getRollupConfiguration } = require('./rollup');

const args = parseArgs({
allowPositionals: true,
options: {
watch: {
type: 'boolean',
description: 'Watch the source files for changes and rebuild when necessary.',
},
},
});

async function main() {
const packageRoot = path.resolve(process.cwd(), args.positionals[0]);

if (!fs.existsSync(packageRoot)) {
console.error(`The package directory "${packageRoot}" does not exist.`);
process.exit(1);
}

if (!fs.existsSync(path.join(packageRoot, 'package.json'))) {
console.error(`The package directory "${packageRoot}" does not contain a package.json file.`);
process.exit(1);
}

const packageData = require(path.join(packageRoot, 'package.json'));
const packageName = packageData.name;
const srcDir = path.join(packageRoot, 'src');
const distDir = path.join(packageRoot, 'dist');

if (!fs.existsSync(srcDir)) {
console.error(`The package directory "${packageRoot}" does not contain a "src" directory.`);
process.exit(1);
}

if (fs.existsSync(distDir)) {
console.log(`Cleaning up the "${distDir}" directory...`);
await fs.promises.rm(distDir, { recursive: true });
await fs.promises.mkdir(distDir);
}

const inputScriptFiles = [
...glob.sync(path.join(srcDir, '*controller.ts')),
...(['@symfony/ux-react', '@symfony/ux-vue', '@symfony/ux-svelte'].includes(packageName)
? [path.join(srcDir, 'loader.ts'), path.join(srcDir, 'components.ts')]
: []),
...(packageName === '@symfony/stimulus-bundle'
? [path.join(srcDir, 'loader.ts'), path.join(srcDir, 'controllers.ts')]
: []),
];

const inputStyleFile = packageData.config?.css_source;
const buildCss = async () => {
const inputStyleFileDist = inputStyleFile
? path.resolve(distDir, `${path.basename(inputStyleFile, '.css')}.min.css`)
: undefined;
if (!inputStyleFile) {
return;
}

console.log('Minifying CSS...');
const css = await fs.promises.readFile(inputStyleFile, 'utf-8');
const minified = new CleanCSS().minify(css).styles;
await fs.promises.writeFile(inputStyleFileDist, minified);
};

if (inputScriptFiles.length === 0) {
console.error(
`No input files found for package "${packageName}" (directory "${packageRoot}").\nEnsure you have at least a file matching the pattern "src/*_controller.ts", or manually specify input files in "${__filename}" file.`
);
process.exit(1);
}

const rollupConfig = getRollupConfiguration({ packageRoot, inputFiles: inputScriptFiles });

if (args.values.watch) {
console.log(
`Watching for JavaScript${inputStyleFile ? ' and CSS' : ''} files modifications in "${srcDir}" directory...`
);

if (inputStyleFile) {
rollupConfig.plugins = (rollupConfig.plugins || []).concat({
name: 'watcher',
buildStart() {
this.addWatchFile(inputStyleFile);
},
});
}

const watcher = rollup.watch(rollupConfig);
watcher.on('event', ({ result }) => {
if (result) {
result.close();
}
});
watcher.on('change', async (id, { event }) => {
if (event === 'update') {
console.log('Files were modified, rebuilding...');
}

if (inputStyleFile && id === inputStyleFile) {
await buildCss();
}
});
} else {
console.log(`Building JavaScript files from ${packageName} package...`);
const start = Date.now();

const bundle = await rollup.rollup(rollupConfig);
await bundle.write(rollupConfig.output);

await buildCss();

console.log(`Done in ${((Date.now() - start) / 1000).toFixed(3)} seconds.`);
}
}

main();
46 changes: 0 additions & 46 deletions bin/build_styles.js

This file was deleted.

134 changes: 134 additions & 0 deletions bin/rollup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
const fs = require('node:fs');
const path = require('node:path');
const resolve = require('@rollup/plugin-node-resolve');
const commonjs = require('@rollup/plugin-commonjs');
const typescript = require('@rollup/plugin-typescript');
const glob = require('glob');

/**
* Guarantees that any files imported from a peer dependency are treated as an external.
*
* For example, if we import `chart.js/auto`, that would not normally
* match the "chart.js" we pass to the "externals" config. This plugin
* catches that case and adds it as an external.
*
* Inspired by https://github.com/oat-sa/rollup-plugin-wildcard-external
*/
const wildcardExternalsPlugin = (peerDependencies) => ({
name: 'wildcard-externals',
resolveId(source, importer) {
if (importer) {
let matchesExternal = false;
peerDependencies.forEach((peerDependency) => {
if (source.includes(`/${peerDependency}/`)) {
matchesExternal = true;
}
});

if (matchesExternal) {
return {
id: source,
external: true,
moduleSideEffects: true,
};
}
}

return null; // other ids should be handled as usually
},
});

/**
* Moves the generated TypeScript declaration files to the correct location.
*
* This could probably be configured in the TypeScript plugin.
*/
const moveTypescriptDeclarationsPlugin = (packageRoot) => ({
name: 'move-ts-declarations',
writeBundle: async () => {
const isBridge = packageRoot.includes('src/Bridge');
const globPattern = path.join('dist', '**', 'assets', 'src', '**/*.d.ts');
const files = glob.sync(globPattern);

files.forEach((file) => {
const relativePath = file;
// a bit odd, but remove first 7 or 4 directories, which will leave only the relative path to the file
// ex: dist/Chartjs/assets/src/controller.d.ts' => 'dist/controller.d.ts'
const targetFile = relativePath.replace(
`${relativePath
.split('/')
.slice(1, isBridge ? 7 : 4)
.join('/')}/`,
''
);
if (!fs.existsSync(path.dirname(targetFile))) {
fs.mkdirSync(path.dirname(targetFile), { recursive: true });
}
fs.renameSync(file, targetFile);
});
},
});

/**
* @param {String} packageRoot
* @param {Array<String>} inputFiles
*/
function getRollupConfiguration({ packageRoot, inputFiles }) {
const packagePath = path.join(packageRoot, 'package.json');
const packageData = JSON.parse(fs.readFileSync(packagePath, 'utf8'));
const peerDependencies = [
'@hotwired/stimulus',
...(packageData.peerDependencies ? Object.keys(packageData.peerDependencies) : []),
];

inputFiles.forEach((file) => {
// custom handling for StimulusBundle
if (file.includes('StimulusBundle/assets/src/loader.ts')) {
peerDependencies.push('./controllers.js');
}

// React, Vue
if (file.includes('assets/src/loader.ts')) {
peerDependencies.push('./components.js');
}
});

const outDir = path.join(packageRoot, 'dist');

return {
input: inputFiles,
output: {
dir: outDir,
entryFileNames: '[name].js',
format: 'esm',
},
external: peerDependencies,
plugins: [
resolve(),
typescript({
filterRoot: '.',
tsconfig: path.join(__dirname, '..', 'tsconfig.json'),
include: [
'src/**/*.ts',
// TODO: Remove for the next major release
// "@rollup/plugin-typescript" v11.0.0 fixed an issue (https://github.com/rollup/plugins/pull/1310) that
// cause a breaking change for UX React users, the dist file requires "react-dom/client" instead of "react-dom"
// and it will break for users using the Symfony AssetMapper without Symfony Flex (for automatic "importmap.php" upgrade).
'**/node_modules/react-dom/client.js',
],
compilerOptions: {
outDir: outDir,
declaration: true,
emitDeclarationOnly: true,
},
}),
commonjs(),
wildcardExternalsPlugin(peerDependencies),
moveTypescriptDeclarationsPlugin(packageRoot),
],
};
}

module.exports = {
getRollupConfiguration,
};
Loading

0 comments on commit 57f7314

Please sign in to comment.