Skip to content

Commit

Permalink
re-export common packages
Browse files Browse the repository at this point in the history
Enable errors for import/no-extraneous-dependencies.

Relying on hoisting makes building Theia application somewhat unstable.
In general cases, everything is fine. But as soon as Yarn decides to
hoist things differently then it can become difficult to fix.

This commit fixes the issue by re-exporting dependencies from
@theia/core, this way instead of relying on implicit dependencies one
can directly import what @theia/core uses like:

    import * as dep from '@theia/core/shared/dep'

To make this work I added a 'shared' folder under 'packages/core' with
js scripts that re-export everything from their matching library.

To ease with the development there is a script named
'packages/core/scripts/generate-shared.js' that will read values
from 'packages/core/package.json#theiaReExports' in order to more easily
add new packages. Depending on how the package exports things we need to
re-export things the same way.

This same script generates the README.md to list all re-exported
packages. It uses README.in.md as a base.

@theia/electron now re-exports all members from electron. This
prevents having to use electron as an implicit dependency.

@theia/electron now also is a @theia/core optional peer dependency. This
directly represent the use case we have for this package where we will
only use electron features if this package is installed by application
makers.

If people don't know about this way to consume @theia/core's shared
dependencies then nothing will change and they might keep using implicit
dependencies. But the new recommended method is to use those re-exports
in order to make builds more stable in the face of hoisting and other
dependency tree optimizations.

Noteworthy: There was an issue with sharing @theia/application-package
since I only re-export the main 'index.js' it tried to include
Node-specific code in frontend applications. I fixed this by
re-exporting @theia/application-package/lib/environment separately.

eslint: add @theia/eslint-plugin package

Add a new package @theia/eslint-plugin to provide rules for downstream
projects to re-use.

The goal was simply to give better error messages if committers used
implicit dependencies again, by notifying when a shared package is
depended upon without going through @theia/core/shared/...

It turned out that implementing that rule revealed places where
import/no-extraneous-dependencies simply didn't catch. The reason might
be that the plugin wasn't designed to work on TS sources. This commit
fixes those newly found occurences of implicit dependencies.

Signed-off-by: Paul <[email protected]>
  • Loading branch information
paul-marechal committed Mar 29, 2021
1 parent 9b98cf9 commit 96930a2
Show file tree
Hide file tree
Showing 932 changed files with 2,302 additions and 1,278 deletions.
1 change: 1 addition & 0 deletions configs/base.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
}
},
"plugins": [
"@theia",
"@typescript-eslint",
"@typescript-eslint/tslint",
"import",
Expand Down
20 changes: 17 additions & 3 deletions configs/errors.eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"$schema": "https://json.schemastore.org/eslintrc",
"rules": {
"@typescript-eslint/consistent-type-definitions": "error",
"@typescript-eslint/indent": "off",
Expand Down Expand Up @@ -34,7 +35,6 @@
"guard-for-in": "error",
"id-blacklist": "off",
"id-match": "off",
"import/no-extraneous-dependencies": "off",
"max-len": [
"error",
{
Expand Down Expand Up @@ -129,6 +129,20 @@
]
}
}
]
}
],
"@theia/shared-dependencies": "error",
"import/no-extraneous-dependencies": "error"
},
"overrides": [
{
"files": [
"dev-packages",
"*.{spec,espec,slow-spec}.{js,ts}"
],
"rules": {
"@theia/shared-dependencies": "off",
"import/no-extraneous-dependencies": "off"
}
}
]
}
3 changes: 2 additions & 1 deletion dev-packages/application-manager/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@
"url-loader": "^1.1.2",
"webpack": "^4.0.0",
"webpack-cli": "2.0.12",
"worker-loader": "^1.1.1"
"worker-loader": "^1.1.1",
"yargs": "^15.3.1"
},
"devDependencies": {
"@theia/ext-scripts": "1.12.0"
Expand Down
4 changes: 2 additions & 2 deletions dev-packages/application-manager/src/expose-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@

import * as path from 'path';
import * as webpack from 'webpack';
// tslint:disable:no-implicit-dependencies
import { RawSourceMap } from 'source-map';
// eslint-disable-next-line import/no-extraneous-dependencies
import type { RawSourceMap } from 'source-map';
import { ApplicationPackage } from '@theia/application-package/lib/application-package';

const modulePackages: { dir: string, name?: string }[] = [];
Expand Down
5 changes: 5 additions & 0 deletions dev-packages/electron/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ The post-install scripts can be skipped by setting an environment variable:
- Windows (cmd): `set THEIA_ELECTRON_SKIP_REPLACE_FFMPEG=1`
- Windows (ps): `$env:THEIA_ELECTRON_SKIP_REPLACE_FFMPEG=1`

## Re-exports

- `electron` through `@theia/electron`
- `native-keymap` through `@theia/electron/native-keymap`

## Additional Information

- [Theia - GitHub](https://github.com/eclipse-theia/theia)
Expand Down
2 changes: 2 additions & 0 deletions dev-packages/electron/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import Electron = require('electron');
export = Electron;
1 change: 1 addition & 0 deletions dev-packages/electron/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = require('electron');
1 change: 1 addition & 0 deletions dev-packages/electron/native-keymap.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from 'native-keymap'
1 change: 1 addition & 0 deletions dev-packages/electron/native-keymap.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = require('native-keymap');
21 changes: 21 additions & 0 deletions dev-packages/eslint-plugin/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// @ts-check
/********************************************************************************
* Copyright (C) 2021 Ericsson and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the Eclipse
* Public License v. 2.0 are satisfied: GNU General Public License, version 2
* with the GNU Classpath Exception which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

/** @type {{[ruleId: string]: import('eslint').Rule.RuleModule}} */
exports.rules = {
"shared-dependencies": require('./rules/shared-dependencies'),
};
9 changes: 9 additions & 0 deletions dev-packages/eslint-plugin/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"private": true,
"name": "@theia/eslint-plugin",
"version": "1.11.0",
"description": "Custom ESLint rules for developing Theia extensions and applications",
"dependencies": {
"@theia/core": "1.11.0"
}
}
171 changes: 171 additions & 0 deletions dev-packages/eslint-plugin/rules/shared-dependencies.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
// @ts-check
/********************************************************************************
* Copyright (C) 2021 Ericsson and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the Eclipse
* Public License v. 2.0 are satisfied: GNU General Public License, version 2
* with the GNU Classpath Exception which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

const fs = require('fs');
const path = require('path');

const {
theiaCoreSharedPrefix,
isSharedModule,
getTheiaCoreSharedModule,
} = require('@theia/core/shared');

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: {
type: "problem",
fixable: 'code',
docs: {
description: 'Errors when a dependency shared by @theia/core is used implicitly, or when a package depends on a shared dependency instead of reusing it from @theia/core/shared. This rule only affects files from packages that depend on @theia/core.',
recommended: true,
},
},
create(context) {
const filename = context.getFilename();
const packageJson = findPackageJson(filename);
if (packageJson && dependsOnTheiaCore(packageJson)) {
// Only show an error regarding the package.json file if this is the first
// time we detect the error, else it will error for every file of the package:
if (firstTime(packageJson.__filename)) {
const sharedModules = getSharedModuleDependencies(packageJson);
if (sharedModules.length > 0) {
context.report({
loc: { line: 0, column: 0 },
message: `"${packageJson.__filename}" depends on some @theia/core shared dependencies: [${sharedModules}]`,
});
}
}
/**
* @param {import('estree').Literal} node
*/
function checkModuleImport(node) {
const module = /** @type {string} */(node.value);
if (isSharedModule(module)) {
context.report({
node,
message: `"${module}" is a @theia/core shared dependency, please use "${theiaCoreSharedPrefix}${module}" instead.`,
fix(fixer) {
if (node.range) {
const [start, end] = node.range;
// Make sure to insert text between the first quote of the string and the rest:
return fixer.insertTextBeforeRange([start + 1, end], theiaCoreSharedPrefix);
}
}
});
} else {
const shared = getTheiaCoreSharedModule(module);
if (shared && !isSharedModule(shared)) {
context.report({
node,
message: `"${shared}" is not part of @theia/core shared dependencies.`
});
}
}
}
return {
ImportDeclaration(node) {
checkModuleImport(node.source);
},
TSExternalModuleReference(node) {
checkModuleImport(node.expression);
},
};
}
return {};
},
};

/** @type {Set<string>} */
const firstTimeCache = new Set();
/**
* @param {string} key
* @returns {boolean} true if first time seeing `key` else false.
*/
function firstTime(key) {
if (firstTimeCache.has(key)) {
return false;
} else {
firstTimeCache.add(key);
return true;
}
}

/**
* @typedef FoundPackageJson
* @property {string} __filename
* @property {{[package: string]: string}} [dependencies]
*/

/**
* Keep a shortcut to a given package.json file based on previous crawls.
* @type {Map<string, FoundPackageJson>}
*/
const findPackageJsonCache = new Map();
/**
* @param {string} from file path to start searching from
* @returns {FoundPackageJson | undefined}
*/
function findPackageJson(from) {
from = path.resolve(from);
let current = fs.statSync(from).isDirectory() ? from : path.dirname(from);
// Keep track of all paths tried before eventually finding a package.json file
const tried = [current];
while (!isRoot(path.parse(from))) {
const cached = findPackageJsonCache.get(current);
if (cached) {
return cached;
}
const packageJsonPath = path.resolve(current, 'package.json');
if (fs.existsSync(packageJsonPath)) {
const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, { encoding: 'utf8' }));
for (const dir of tried) {
findPackageJsonCache.set(dir, packageJson);
}
packageJson['__filename'] = packageJsonPath;
return packageJson;
}
current = path.dirname(current);
tried.push(current);
}
}

/**
* @param {path.ParsedPath} parsed
* @returns {boolean}
*/
function isRoot(parsed) {
return parsed.base === '' && parsed.dir === parsed.root;
}

/**
* @param {object} packageJson
* @returns {boolean}
*/
function dependsOnTheiaCore(packageJson) {
return typeof packageJson.dependencies === 'object'
&& '@theia/core' in packageJson.dependencies;
}

/**
* @param {object} packageJson
* @return {string[]}
*/
function getSharedModuleDependencies(packageJson) {
return typeof packageJson.dependencies === 'object'
? Object.keys(packageJson.dependencies).filter(dependency => isSharedModule(dependency))
: [];
}
6 changes: 6 additions & 0 deletions examples/api-samples/compile.tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@
},
{
"path": "../../packages/vsx-registry/compile.tsconfig.json"
},
{
"path": "../../packages/filesystem/compile.tsconfig.json"
},
{
"path": "../../packages/workspace/compile.tsconfig.json"
}
]
}
2 changes: 2 additions & 0 deletions examples/api-samples/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
"description": "Theia - Example code to demonstrate Theia API",
"dependencies": {
"@theia/core": "1.12.0",
"@theia/filesystem": "1.12.0",
"@theia/output": "1.12.0",
"@theia/workspace": "1.12.0",
"@theia/vsx-registry": "1.12.0"
},
"theiaExtensions": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { ContainerModule } from 'inversify';
import { ContainerModule } from '@theia/core/shared/inversify';
import { bindDynamicLabelProvider } from './label/sample-dynamic-label-provider-command-contribution';
import { bindSampleUnclosableView } from './view/sample-unclosable-view-contribution';
import { bindSampleOutputChannelWithSeverity } from './output/sample-output-channel-with-severity';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { postConstruct, injectable, inject, interfaces } from 'inversify';
import { postConstruct, injectable, inject, interfaces } from '@theia/core/shared/inversify';
import {
createPreferenceProxy, FrontendApplicationContribution, LabelProvider,
PreferenceContribution, PreferenceProxy, PreferenceSchema, PreferenceService
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { injectable, inject, interfaces } from 'inversify';
import { injectable, inject, interfaces } from '@theia/core/shared/inversify';
import { Command, CommandContribution, CommandRegistry, CommandHandler } from '@theia/core';
import { FrontendApplicationContribution, LabelProviderContribution } from '@theia/core/lib/browser';
import { SampleDynamicLabelProviderContribution } from './sample-dynamic-label-provider-contribution';
Expand Down Expand Up @@ -59,4 +59,3 @@ export const bindDynamicLabelProvider = (bind: interfaces.Bind) => {
bind(LabelProviderContribution).toService(SampleDynamicLabelProviderContribution);
bind(CommandContribution).to(SampleDynamicLabelProviderCommandContribution).inSingletonScope();
};

Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { injectable } from 'inversify';
import { injectable } from '@theia/core/shared/inversify';
import { DefaultUriLabelProviderContribution, DidChangeLabelEvent } from '@theia/core/lib/browser/label-provider';
import URI from '@theia/core/lib/common/uri';
import { Emitter, Event } from '@theia/core';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { injectable, ContainerModule } from 'inversify';
import { Menu as MenuWidget } from '@phosphor/widgets';
import { injectable, ContainerModule } from '@theia/core/shared/inversify';
import { Menu as MenuWidget } from '@theia/core/shared/@phosphor/widgets';
import { Disposable } from '@theia/core/lib/common/disposable';
import { MenuNode, CompositeMenuNode } from '@theia/core/lib/common/menu';
import { BrowserMainMenuFactory, MenuCommandRegistry, DynamicMenuWidget } from '@theia/core/lib/browser/menu/browser-menu-plugin';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
********************************************************************************/

import { Command, CommandContribution, CommandRegistry, MAIN_MENU_BAR, MenuContribution, MenuModelRegistry, MenuNode, SubMenuOptions } from '@theia/core/lib/common';
import { injectable, interfaces } from 'inversify';
import { injectable, interfaces } from '@theia/core/shared/inversify';

const SampleCommand: Command = {
id: 'sample-command',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
import { FrontendApplicationContribution } from '@theia/core/lib/browser';
import { inject, injectable, interfaces } from 'inversify';
import { inject, injectable, interfaces } from '@theia/core/shared/inversify';
import { OutputChannelManager, OutputChannelSeverity } from '@theia/output/lib/common/output-channel';

@injectable()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { inject, injectable, interfaces } from 'inversify';
import { inject, injectable, interfaces } from '@theia/core/shared/inversify';
import { AbstractViewContribution, bindViewContribution } from '@theia/core/lib/browser/shell/view-contribution';
import { TabBarToolbarContribution, TabBarToolbarRegistry } from '@theia/core/lib/browser/shell/tab-bar-toolbar';
import { Command, CommandRegistry, MessageService } from '@theia/core/lib/common';
Expand Down
Loading

0 comments on commit 96930a2

Please sign in to comment.