From 0fc2a0b504653bea66eb34d7f235a602cfcedc5c Mon Sep 17 00:00:00 2001 From: Dan Freeman Date: Mon, 20 Jan 2020 13:10:43 -0500 Subject: [PATCH] fix: issue a warning if we detect a .js/.ts file collision --- package.json | 3 ++ tests/dummy/app/shadowed-file.ts | 5 -- .../dummy/lib/in-repo-a/app/shadowed-file.ts | 4 -- .../dummy/lib/in-repo-b/app/shadowed-file.js | 4 -- ts/addon.ts | 52 ++++++++++++++++++- ts/tests/acceptance/build-test.ts | 27 ++++++++++ ts/tests/helpers/skeleton-app.ts | 2 +- ts/types/broccoli-stew/index.d.ts | 6 +++ .../ember-cli-preprocess-registry/index.d.ts | 17 ++++++ ts/types/ember-cli/index.d.ts | 3 +- yarn.lock | 33 +++++++++++- 11 files changed, 138 insertions(+), 18 deletions(-) delete mode 100644 tests/dummy/app/shadowed-file.ts delete mode 100644 tests/dummy/lib/in-repo-a/app/shadowed-file.ts delete mode 100644 tests/dummy/lib/in-repo-b/app/shadowed-file.js create mode 100644 ts/types/broccoli-stew/index.d.ts create mode 100644 ts/types/ember-cli-preprocess-registry/index.d.ts diff --git a/package.json b/package.json index 198f99fd3..24aff91fa 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ "@babel/plugin-proposal-optional-chaining": "^7.6.0", "@babel/plugin-transform-typescript": "~7.7.0", "ansi-to-html": "^0.6.6", + "broccoli-stew": "^3.0.0", "debug": "^4.0.0", "ember-cli-babel-plugin-helpers": "^1.0.0", "execa": "^3.0.0", @@ -79,6 +80,8 @@ "@typescript-eslint/eslint-plugin": "2.12.0", "@typescript-eslint/parser": "2.12.0", "broccoli-asset-rev": "3.0.0", + "broccoli-node-api": "^1.7.0", + "broccoli-plugin": "^4.0.1", "capture-console": "1.0.1", "co": "4.6.0", "commitlint-azure-pipelines-cli": "1.0.2", diff --git a/tests/dummy/app/shadowed-file.ts b/tests/dummy/app/shadowed-file.ts deleted file mode 100644 index a94ed3508..000000000 --- a/tests/dummy/app/shadowed-file.ts +++ /dev/null @@ -1,5 +0,0 @@ -// This file should win out over `shadowed-file.ts` and `shadowed-file.js` -// in our two in-repo addons. -const value: string = 'dummy/shadowed-file'; - -export default value; diff --git a/tests/dummy/lib/in-repo-a/app/shadowed-file.ts b/tests/dummy/lib/in-repo-a/app/shadowed-file.ts deleted file mode 100644 index d04e69fc8..000000000 --- a/tests/dummy/lib/in-repo-a/app/shadowed-file.ts +++ /dev/null @@ -1,4 +0,0 @@ -// This should be shadowed by the dummy app's own `shadowed-file.ts` -const value: string = 'bad'; - -export default value; diff --git a/tests/dummy/lib/in-repo-b/app/shadowed-file.js b/tests/dummy/lib/in-repo-b/app/shadowed-file.js deleted file mode 100644 index ded27bc8e..000000000 --- a/tests/dummy/lib/in-repo-b/app/shadowed-file.js +++ /dev/null @@ -1,4 +0,0 @@ -// This should be shadowed by the dummy app's own `sahdowed-file.ts` -const value = 'bad'; - -export default value; diff --git a/ts/addon.ts b/ts/addon.ts index 929288c82..de2f645a7 100644 --- a/ts/addon.ts +++ b/ts/addon.ts @@ -3,6 +3,7 @@ import { Remote } from 'stagehand'; import { connect } from 'stagehand/lib/adapters/child-process'; import { hasPlugin, addPlugin, AddPluginOptions } from 'ember-cli-babel-plugin-helpers'; import Addon from 'ember-cli/lib/models/addon'; +import PreprocessRegistry from 'ember-cli-preprocess-registry'; import { addon } from './lib/utilities/ember-cli-entities'; import fork from './lib/utilities/fork'; import TypecheckWorker from './lib/typechecking/worker'; @@ -18,6 +19,7 @@ export default addon({ included() { this._super.included.apply(this, arguments); + this._checkDevelopment(); this._checkAddonAppFiles(); this._checkBabelVersion(); @@ -69,9 +71,16 @@ export default addon({ } }, - setupPreprocessorRegistry(type) { + setupPreprocessorRegistry(type, registry) { if (type !== 'parent') return; + // If we're acting on behalf of the root app, issue a warning if we detect + // a situation where a .js file from an addon has the same name as a .ts + // file in the app, as which file wins is nondeterministic. + if (this.parent === this.project) { + this._registerCollisionDetectionPreprocessor(registry); + } + // Normally this is the sort of logic that would live in `included()`, but // ember-cli-babel reads the configured extensions when setting up the // preprocessor registry, so we need to beat it to the punch. @@ -97,6 +106,47 @@ export default addon({ return !['in-repo-a', 'in-repo-b'].includes(addon.name); }, + _registerCollisionDetectionPreprocessor(registry: PreprocessRegistry) { + registry.add('js', { + name: 'ember-cli-typescript-collision-check', + toTree: (input, path) => { + if (path !== '/') return input; + + let addon = this; + let checked = false; + let stew = require('broccoli-stew') as typeof import('broccoli-stew'); + + return stew.afterBuild(input, function() { + if (!checked) { + checked = true; + addon._checkForFileCollisions(this.inputPaths[0]); + } + }); + }, + }); + }, + + _checkForFileCollisions(directory: string) { + let walkSync = require('walk-sync') as typeof import('walk-sync'); + let files = new Set(walkSync(directory, ['**/*.{js,ts}'])); + + let collisions = []; + for (let file of files) { + if (file.endsWith('.js') && files.has(file.replace(/\.js$/, '.ts'))) { + collisions.push(file.replace(/\.js$/, '.{js,ts}')); + } + } + + if (collisions.length) { + this.ui.writeWarnLine( + 'Detected collisions between .js and .ts files of the same name. ' + + 'This can result in nondeterministic build output; ' + + 'see https://git.io/JvIwo for more information.\n - ' + + collisions.join('\n - ') + ); + } + }, + _checkBabelVersion() { let babel = this.parent.addons.find(addon => addon.name === 'ember-cli-babel'); let version = babel && babel.pkg.version; diff --git a/ts/tests/acceptance/build-test.ts b/ts/tests/acceptance/build-test.ts index e2f73890e..10ff9a233 100644 --- a/ts/tests/acceptance/build-test.ts +++ b/ts/tests/acceptance/build-test.ts @@ -101,6 +101,33 @@ describe('Acceptance: build', function() { await server.waitForBuild(); }); + + it('emits a warning when .js and .ts files conflict in the app/ tree', async () => { + // Set up an in-repo addon + app.updatePackageJSON(pkg => { + pkg['ember-addon'].paths.push('lib/in-repo-addon'); + }); + + app.writeFile('lib/in-repo-addon/index.js', 'module.exports = { name: "in-repo-addon" };'); + app.writeFile( + 'lib/in-repo-addon/package.json', + JSON.stringify({ + name: 'in-repo-addon', + keywords: ['ember-addon'], + }) + ); + + // Have it export a .js app file and attempt to overwrite it in the host with a .ts file + app.writeFile('lib/in-repo-addon/app/foo.js', '// addon'); + app.writeFile('app/foo.ts', '// app'); + + let output = await app.build(); + + expect(output.all).to.include('skeleton-app/foo.{js,ts}'); + expect(output.all).to.include( + 'WARNING: Detected collisions between .js and .ts files of the same name.' + ); + }); }); function isExpressionStatement(stmt: Statement | ModuleDeclaration): stmt is ExpressionStatement { diff --git a/ts/tests/helpers/skeleton-app.ts b/ts/tests/helpers/skeleton-app.ts index 845d4d759..e5435c549 100644 --- a/ts/tests/helpers/skeleton-app.ts +++ b/ts/tests/helpers/skeleton-app.ts @@ -44,7 +44,7 @@ export default class SkeletonApp { )); } - updatePackageJSON(callback: (arg: any) => string) { + updatePackageJSON(callback: (arg: any) => any) { let pkgPath = `${this.root}/package.json`; let pkg = fs.readJSONSync(pkgPath); fs.writeJSONSync(pkgPath, callback(pkg) || pkg, { spaces: 2 }); diff --git a/ts/types/broccoli-stew/index.d.ts b/ts/types/broccoli-stew/index.d.ts new file mode 100644 index 000000000..8e261c2cc --- /dev/null +++ b/ts/types/broccoli-stew/index.d.ts @@ -0,0 +1,6 @@ +declare module 'broccoli-stew' { + import { Node as BroccoliNode } from 'broccoli-node-api'; + import Plugin from 'broccoli-plugin'; + + export function afterBuild(node: BroccoliNode, callback: (this: Plugin) => void): BroccoliNode; +} diff --git a/ts/types/ember-cli-preprocess-registry/index.d.ts b/ts/types/ember-cli-preprocess-registry/index.d.ts new file mode 100644 index 000000000..4e6b3dace --- /dev/null +++ b/ts/types/ember-cli-preprocess-registry/index.d.ts @@ -0,0 +1,17 @@ +declare module 'ember-cli-preprocess-registry' { + import { Node as BroccoliNode } from 'broccoli-node-api'; + + export = PreprocessRegistry; + + class PreprocessRegistry { + add(type: string, plugin: PreprocessPlugin): void; + load(type: string): Array; + extensionsForType(type: string): Array; + remove(type: string, plugin: PreprocessPlugin): void; + } + + interface PreprocessPlugin { + name: string; + toTree(input: BroccoliNode, path: string): BroccoliNode; + } +} diff --git a/ts/types/ember-cli/index.d.ts b/ts/types/ember-cli/index.d.ts index 605a64fff..1c78069ef 100644 --- a/ts/types/ember-cli/index.d.ts +++ b/ts/types/ember-cli/index.d.ts @@ -13,6 +13,7 @@ declare module 'ember-cli/lib/models/addon' { import Project from 'ember-cli/lib/models/project'; import Command from 'ember-cli/lib/models/command'; import EmberApp from 'ember-cli/lib/broccoli/ember-app'; + import PreprocessRegistry from 'ember-cli-preprocess-registry'; export default class Addon extends CoreObject { name: string; @@ -37,7 +38,7 @@ declare module 'ember-cli/lib/models/addon' { isDevelopingAddon(): boolean; serverMiddleware(options: { app: Application }): void | Promise; testemMiddleware(app: Application): void; - setupPreprocessorRegistry(type: 'self' | 'parent', registry: unknown): void; + setupPreprocessorRegistry(type: 'self' | 'parent', registry: PreprocessRegistry): void; } } diff --git a/yarn.lock b/yarn.lock index bb71ca224..f342bea73 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4018,6 +4018,15 @@ broccoli-output-wrapper@^2.0.0: dependencies: heimdalljs-logger "^0.1.10" +broccoli-output-wrapper@^3.1.1: + version "3.2.0" + resolved "https://registry.yarnpkg.com/broccoli-output-wrapper/-/broccoli-output-wrapper-3.2.0.tgz#6340e92f5ac81c6f85e6197af822c21628933f35" + integrity sha512-cYXh5+h+UMCz68fJYOYDT2iNlTUUELfkTceDoxXio3YosxoOrxiBfqCLnxDd80twXCUkiq4W1v8AtTzLBO4Vjg== + dependencies: + fs-extra "^8.1.0" + heimdalljs-logger "^0.1.10" + symlink-or-copy "^1.2.0" + broccoli-persistent-filter@^1.1.5, broccoli-persistent-filter@^1.1.6, broccoli-persistent-filter@^1.2.0, broccoli-persistent-filter@^1.4.3: version "1.4.6" resolved "https://registry.yarnpkg.com/broccoli-persistent-filter/-/broccoli-persistent-filter-1.4.6.tgz#80762d19000880a77da33c34373299c0f6a3e615" @@ -4111,6 +4120,19 @@ broccoli-plugin@^3.0.0: rimraf "^2.3.4" symlink-or-copy "^1.1.8" +broccoli-plugin@^4.0.1: + version "4.0.1" + resolved "https://registry.yarnpkg.com/broccoli-plugin/-/broccoli-plugin-4.0.1.tgz#5a0468a9c8e02f763d5c162ced0a5930db4567a9" + integrity sha512-rBYVtV1rWvlDS8fd+CUUG7L/TO5VUCRjaGm2HEOBaTwUYQKswKJXLRSxwv0CYLo3QfVZJpI1akcn7NGe9kywIQ== + dependencies: + broccoli-node-api "^1.6.0" + broccoli-output-wrapper "^3.1.1" + fs-merger "^3.0.1" + promise-map-series "^0.2.1" + quick-temp "^0.1.3" + rimraf "^3.0.0" + symlink-or-copy "^1.3.0" + broccoli-postcss-single@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/broccoli-postcss-single/-/broccoli-postcss-single-3.0.0.tgz#1393f87b69e4bbf20a1a1e614e09bbf066db28a2" @@ -9747,10 +9769,12 @@ imurmurhash@^0.1.4: integrity sha1-khi5srkoojixPcT7a21XbyMUU+o= "in-repo-a@link:tests/dummy/lib/in-repo-a": - version "1.0.0" + version "0.0.0" + uid "" "in-repo-b@link:tests/dummy/lib/in-repo-b": - version "1.0.0" + version "0.0.0" + uid "" include-path-searcher@^0.1.0: version "0.1.0" @@ -15459,6 +15483,11 @@ symlink-or-copy@^1.0.0, symlink-or-copy@^1.0.1, symlink-or-copy@^1.1.8, symlink- resolved "https://registry.yarnpkg.com/symlink-or-copy/-/symlink-or-copy-1.2.0.tgz#5d49108e2ab824a34069b68974486c290020b393" integrity sha512-W31+GLiBmU/ZR02Ii0mVZICuNEN9daZ63xZMPDsYgPgNjMtg+atqLEGI7PPI936jYSQZxoLb/63xos8Adrx4Eg== +symlink-or-copy@^1.3.0: + version "1.3.1" + resolved "https://registry.yarnpkg.com/symlink-or-copy/-/symlink-or-copy-1.3.1.tgz#9506dd64d8e98fa21dcbf4018d1eab23e77f71fe" + integrity sha512-0K91MEXFpBUaywiwSSkmKjnGcasG/rVBXFLJz5DrgGabpYD6N+3yZrfD6uUIfpuTu65DZLHi7N8CizHc07BPZA== + sync-disk-cache@^1.3.3: version "1.3.3" resolved "https://registry.yarnpkg.com/sync-disk-cache/-/sync-disk-cache-1.3.3.tgz#481933461623fdc2bdf46cfc87872ba215a7e246"