Skip to content

Issue a warning if we detect a .js/.ts file collision #1046

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

Merged
merged 3 commits into from
Jan 21, 2020
Merged
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
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"@babel/plugin-proposal-optional-chaining": "^7.6.0",
"@babel/plugin-transform-typescript": "~7.8.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",
Expand Down Expand Up @@ -79,6 +80,8 @@
"@typescript-eslint/eslint-plugin": "2.17.0",
"@typescript-eslint/parser": "2.17.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.3",
Expand Down
5 changes: 0 additions & 5 deletions tests/dummy/app/shadowed-file.ts

This file was deleted.

4 changes: 0 additions & 4 deletions tests/dummy/lib/in-repo-a/app/shadowed-file.ts

This file was deleted.

4 changes: 0 additions & 4 deletions tests/dummy/lib/in-repo-b/app/shadowed-file.js

This file was deleted.

52 changes: 51 additions & 1 deletion ts/addon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -18,6 +19,7 @@ export default addon({

included() {
this._super.included.apply(this, arguments);

this._checkDevelopment();
this._checkAddonAppFiles();
this._checkBabelVersion();
Expand Down Expand Up @@ -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.
Expand All @@ -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;
Expand Down
27 changes: 27 additions & 0 deletions ts/tests/acceptance/build-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion ts/tests/helpers/skeleton-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
6 changes: 6 additions & 0 deletions ts/types/broccoli-stew/index.d.ts
Original file line number Diff line number Diff line change
@@ -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;
}
17 changes: 17 additions & 0 deletions ts/types/ember-cli-preprocess-registry/index.d.ts
Original file line number Diff line number Diff line change
@@ -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<PreprocessPlugin>;
extensionsForType(type: string): Array<string>;
remove(type: string, plugin: PreprocessPlugin): void;
}

interface PreprocessPlugin {
name: string;
toTree(input: BroccoliNode, path: string): BroccoliNode;
}
}
3 changes: 2 additions & 1 deletion ts/types/ember-cli/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,7 +38,7 @@ declare module 'ember-cli/lib/models/addon' {
isDevelopingAddon(): boolean;
serverMiddleware(options: { app: Application }): void | Promise<void>;
testemMiddleware(app: Application): void;
setupPreprocessorRegistry(type: 'self' | 'parent', registry: unknown): void;
setupPreprocessorRegistry(type: 'self' | 'parent', registry: PreprocessRegistry): void;
}
}

Expand Down
33 changes: 31 additions & 2 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4267,6 +4267,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"
Expand Down Expand Up @@ -4360,6 +4369,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"
Expand Down Expand Up @@ -10194,10 +10216,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"
Expand Down Expand Up @@ -16182,6 +16206,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"
Expand Down