# RFC: Enforce consistency through package linting

## Problem

When packages have assumptions about how they are consumed, or how they should be formatted, it makes them complicated to consume in a standard way. It is easy for developers to miss details when publishing their packages that make it hard to standardize consumption.

Cloudpack is an effort to standardize providing on-demand es module bundles per npm package/version. These bundles can be consumed in the browser and can be serviced through an import-map service.

For Cloudpack to create isolated bundles for any npm package, we need packages to follow a set of conformance rules:

1. They must include ES modules. (While CommonJS is supported for old node scenarios, it creates complexities for bundlers; cjs can't be tree shaken, and for isolated esm package bundling, cjs is not explicit about named exports and so we end up parsing the code to generate esm entry point stubs.)
2. They must be explicit about `sideEffects` in package.json. Without this, we can't tree shake bundles properly.
3. They must have no deep imports from other packages which aren't expressed as valid entries in `exports`. (Reaching into private things creates non-enforced contracts and leads to isolated bundles that contain duplicates of singletons.)
4. They must have no proprietary dependencies on bundler loaders which aren't supported. (but we can support a subset of standard import types like json, sass, css, txt, md, json, graphql)
5. They should be`isolatedModules` compliant. This is a TypeScript setting which indicates the package is built to be transpiled in isolation. (This implies they use `import type` and `export type` when dealing with types, and avoid using const enums.)
6. They should have TypeScript types referenced in the package.json. Otherwise they can't be consumed by other TypeScript libraries.
7. They should avoid using star exports from other libraries. Nested cross-package star exports are not preserved in either esbuild or webpack esm output.
9. They must follow semantic versioning (which should be enfored through api extractor in connection with beachball.)
10. Dependencies should be referenced correctly (caret deps) and precisely (all referenced packages listed, no additional unused packages listed.) Otherwise we are more likely pulling in duplicate dependencies.

This list is non-exhaustive, yet we don't validate any of these today and have no common tooling to enforce these or to stage and fix new validation.

When Microsoft produces packages which violate these rules, it pushes problems downstream, often manually mitigated or ignored. This creates numerous friction points without fixing root causes. The goal of this investigate is to address the root causes through common conformance validation.

## Assessing solutions

Many of these issues really need to be validated after building, but before publishing. A project may have multiple build steps which do things like produce multiple module formats, generate stubs for proprietary import types, or roll the output up into a common file. We want validation to occur when these steps are completed, but before being published. That way we can ensure things can be consumed properly downstream.

We don't want to overlap with existing tools. Some existing linting tools:

* boll
* npm-package-json-linter
* eslint

## Ideas on how to address

We need the ability to quickly iterate and update rules as we find problems. Today we have eslint, which can guard against code problems. For the remaining publishing issues, we have the following options:

### Idea 1: invent a specifically-scoped tool

Ideally we have a cli tool hosted in the microsoft/1js-tools github repo which can be integrated in multiple build toolchains. This would enable us to use whatever we want under the hood; perhaps a combination of `npm-package-json-linter` and specific `eslint` rules would be used to validate the package.

### Idea 2: take over boll and repurpose specifically for pre-publishing validation

Boll is an internal Microsoft project that loosely relates to the problem. The project lost ownership and momentum when the author left the company.

Boll's current scope and future is murky at best. One idea would be to leverage the existing code and make it more focused to solve the pre-publishing concerns that can't be solved with eslint.

Likely we'd rename, gut and repurpose it. This way existing projects using the current version/validation won't break.

### Idea 2: add to npm-package-json-lint

### Idea 3: fork npm-package-json-lint

## Reference: tooling evaluation

### npm-package-json-lint

https://npmpackagejsonlint.org/

"A configurable linter for package.json files"

https://github.com/tclindner/npm-package-json-lint/tree/master/src

This tool helps validate package.json conformance, but is more of a glorified schema checker rather than a validator. For example, a "require-module" rule exists, but doesn't actually validate that the module is valid and points to esm. There's a "require-bugs" rule, but doesn't actually validate the url is valid and points to a github or vso repo.

The project is in active development with the last checkin coming in 12 days ago. Source is built in JavaScript, rather than in TypeScript.

Similar to eslint, requires a config which can extend a base, but you can override things. This means we can do things like change our base to require "bugs" be populated, and auto suppress the rule on tooling updates.

Lots of rules available:

"require-author": "error",
"require-description": "error",
"require-engines": "error",
"require-license": "error",
"require-name": "error",
"require-repository": "error",
"require-version": "error",
"require-bugs": "error",
"require-homepage": "error",
"require-keywords": "error",
"bin-type": "error",
"config-type": "error",
"description-type": "error",
"devDependencies-type": "error",
"directories-type": "error",
"engines-type": "error",
"files-type": "error",
"homepage-type": "error",
"keywords-type": "error",
"license-type": "error",
"main-type": "error",
"man-type": "error",
"name-type": "error",
"preferGlobal-type": "error",
"private-type": "error",
"repository-type": "error",
"scripts-type": "error",
"version-type": "error",

### Boll

Boll is a Microsoft tool used in Office, which lints a variety of things. It is unclear when Boll runs, but it seems to be at linting time, rather than post-install.

It is also unclear what it is and isn't responsible for. The tool is built as a "catch all" linter, where some rules apply to the TypeScript (why not typescript-eslint?) and some rules apply to package.json (why not package-lint?)

The project has ceased development as the primary author has left Microsoft and stopped contributing and managing the project.

To gain a better understanding of its coverage, here are the rules it currently supports:

- `CrossPackageDependencyDetector` - detects usage of files stored directly in other packages that happen to be in a known location on disk. (E.g. `import foo from '../../../../foo-package/lib/foo')`)`
- `NodeModulesReferencesDetector` - detects imports to `node_modules` paths
- `RedundantImportsDetector` - detects imports that are redundant. (Should be an eslint rule.)
- `SrcDetector` - detects usage of `src` in import statements of TypeScript source files. (Seems to imply the tool runs on source code rather than published code.)
- `TransitiveDependencyDetector` - detects imports to packages which aren't explicitly listed as dependencies within `package.json`.
- `ESLintPreferConstRule` - ensures that the prefer-const eslint rule is enabled on all source file and that the rule is enabled as an error. (Why is this needed? What is special about this rule that requires another tool to validate it's on?)
- `NoRedundantDepsRule` - ensures that peerDepenendecies in `package.json` aren't re-declared as regular dependencies.
- `EnforceRationale` - ensures specific fields in package.json require a rationale for any additions. (Unclear how this works with `json` files that can't have comments.)
- `PackageConsistency`

### npm-pkg-lint

https://github.com/ext/npm-pkg-lint

2 stars, not used. Last code update a month ago, 1 contributor, written in TypeScript. However, there are good things about it to leverage:

* disallowed files check (typescript config, unit tests, code coverage reports, test files, ci-related files). Should this just be an npmignore validator?
* Missing files - verifies that entries in package.json actually exist. This is good. But doens't validate their validity
* disallowed dependencies - verifies that common dev tools like eslint, typescript, etc are not listed as dependencies. Might be good, but even better would be to do something like complain about unused dependencies after attempting to bundle assets. If you include 'lodash' due to copy/paste but never use it, it's similar to the problem of including `typescript` in dependencies.
* Shebang - validate bin files has `#/usr/bin/env node` at the beginning of a file
* `package.json` fields - validates description, keywords, etc are present. Also enforces urls to be https, rather than http.
* verify engine constraints - validates all transitive dependencies satisfy the node requirement.
* @types/node and engine constraints - requires engines.node lowest major version is equal to `@types/node` major version. This is really smart.