Skip to content
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

[ENHANCEMENT] Add support for --typescript flag to app and addon blueprints #9972

Merged
merged 16 commits into from Oct 5, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 12 additions & 1 deletion blueprints/addon/index.js
Expand Up @@ -29,6 +29,8 @@ module.exports = {
description,
appBlueprintName: 'app',

shouldTransformTypeScript: true,

filesToRemove: [
'tests/dummy/app/styles/.gitkeep',
'tests/dummy/app/templates/.gitkeep',
Expand Down Expand Up @@ -100,13 +102,20 @@ module.exports = {
this.ui.writeLine(prependEmoji('✨', `Creating a new Ember addon in ${chalk.yellow(process.cwd())}:`));
},

afterInstall() {
async afterInstall(options) {
let packagePath = path.join(this.path, 'files', 'package.json');
let bowerPath = path.join(this.path, 'files', 'bower.json');

[packagePath, bowerPath].forEach((filePath) => {
fs.removeSync(filePath);
});

if (options.typescript) {
await this.addAddonToProject({
name: 'ember-cli-typescript',
blueprintOptions: { ...options, save: true },
});
}
},

locals(options) {
Expand All @@ -132,6 +141,7 @@ module.exports = {
options.welcome && '"--welcome"',
options.yarn && '"--yarn"',
options.ciProvider && `"--ci-provider=${options.ciProvider}"`,
options.typescript && `"--typescript"`,
]
.filter(Boolean)
.join(',\n ') +
Expand All @@ -154,6 +164,7 @@ module.exports = {
embroider: false,
lang: options.lang,
ciProvider: options.ciProvider,
typescript: options.typescript,
};
},

Expand Down
2 changes: 1 addition & 1 deletion blueprints/app/files/.ember-cli
Expand Up @@ -11,5 +11,5 @@
Setting `isTypeScriptProject` to true will force the blueprint generators to generate TypeScript
rather than JavaScript by default, when a TypeScript version of a given blueprint is available.
*/
"isTypeScriptProject": false
"isTypeScriptProject": <%= typescript ? 'true' : 'false' %>
}
15 changes: 12 additions & 3 deletions blueprints/app/files/.eslintrc.js
Expand Up @@ -2,15 +2,15 @@

module.exports = {
root: true,
parser: 'babel-eslint',
parser: '<%= typescript ? '@typescript-eslint/parser' : 'babel-eslint' %>',
parserOptions: {
ecmaVersion: 2018,
sourceType: 'module',
ecmaFeatures: {
legacyDecorators: true,
},
},
plugins: ['ember'],
plugins: ['ember'<% if (typescript) { %>, '@typescript-eslint'<% } %>],
extends: [
'eslint:recommended',
'plugin:ember/recommended',
Expand All @@ -21,7 +21,16 @@ module.exports = {
},
rules: {},
overrides: [
// node files
<% if (typescript) { %> // ts files
{
files: ['**/*.ts'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this has been changed/fixed in more recent versions of ESLint, but the last I checked, a glob like this would still require you to specify --ext .js,.ts when executing the linter. I've found just '*.ts' still enables linting for nested files while also being simple enough for ESLint to understand it should automatically lint .ts files as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have this config in a few addons/projects without any problems! 🤷‍♂️

extends: [
'plugin:@typescript-eslint/eslint-recommended',
'plugin:@typescript-eslint/recommended',
],
rules: {},
},
<% } %> // node files
{
files: [
'./.eslintrc.js',
Expand Down
File renamed without changes.
Expand Up @@ -6,4 +6,6 @@ export default class Router extends EmberRouter {
rootURL = config.rootURL;
}

Router.map(function () {});
Router.map(function () {<% if (typescript) { %>
// Add route declarations here
<% } %>});
9 changes: 6 additions & 3 deletions blueprints/app/files/package.json
Expand Up @@ -17,7 +17,8 @@
"lint:hbs": "ember-template-lint .",
"lint:hbs:fix": "ember-template-lint . --fix",
"lint:js": "eslint . --cache",
"lint:js:fix": "eslint . --fix",
"lint:js:fix": "eslint . --fix",<% if (typescript) { %>
"lint:types": "tsc --noEmit",<% } %>
"start": "ember serve",
"test": "concurrently \"npm:lint\" \"npm:test:*\" --names \"lint,test:\"",
"test:ember": "ember test"
Expand All @@ -29,8 +30,10 @@
"@embroider/core": "^1.8.3",
"@embroider/webpack": "^1.8.3<% } %>",
"@glimmer/component": "^1.1.2",
"@glimmer/tracking": "^1.1.2",
"babel-eslint": "^10.1.0",
"@glimmer/tracking": "^1.1.2",<% if (typescript) { %>
"@typescript-eslint/eslint-plugin": "^5.30.5",
"@typescript-eslint/parser": "^5.30.5",<% } else { %>
"babel-eslint": "^10.1.0",<% } %>
"broccoli-asset-rev": "^3.0.0",
"concurrently": "^7.4.0",
"ember-auto-import": "^2.4.2",
Expand Down
43 changes: 43 additions & 0 deletions blueprints/app/files/tests/helpers/index.ts
@@ -0,0 +1,43 @@
import {
setupApplicationTest as upstreamSetupApplicationTest,
setupRenderingTest as upstreamSetupRenderingTest,
setupTest as upstreamSetupTest,
SetupTestOptions,
} from 'ember-qunit';

// This file exists to provide wrappers around ember-qunit's / ember-mocha's
// test setup functions. This way, you can easily extend the setup that is
// needed per test type.

function setupApplicationTest(hooks: NestedHooks, options?: SetupTestOptions) {
upstreamSetupApplicationTest(hooks, options);

// Additional setup for application tests can be done here.
//
// For example, if you need an authenticated session for each
// application test, you could do:
//
// hooks.beforeEach(async function () {
// await authenticateSession(); // ember-simple-auth
// });
//
// This is also a good place to call test setup functions coming
// from other addons:
//
// setupIntl(hooks); // ember-intl
// setupMirage(hooks); // ember-cli-mirage
}

function setupRenderingTest(hooks: NestedHooks, options?: SetupTestOptions) {
upstreamSetupRenderingTest(hooks, options);

// Additional setup for rendering tests can be done here.
}

function setupTest(hooks: NestedHooks, options?: SetupTestOptions) {
upstreamSetupTest(hooks, options);

// Additional setup for unit tests can be done here.
}

export { setupApplicationTest, setupRenderingTest, setupTest };
10 changes: 10 additions & 0 deletions blueprints/app/index.js
Expand Up @@ -8,6 +8,8 @@ const directoryForPackageName = require('../../lib/utilities/directory-for-packa
module.exports = {
description: 'The default blueprint for ember-cli projects.',

shouldTransformTypeScript: true,

filesToRemove: [
'app/styles/.gitkeep',
'app/templates/.gitkeep',
Expand Down Expand Up @@ -37,6 +39,7 @@ module.exports = {
options.yarn && '"--yarn"',
embroider && '"--embroider"',
options.ciProvider && `"--ci-provider=${options.ciProvider}"`,
options.typescript && `"--typescript"`,
]
.filter(Boolean)
.join(',\n ') +
Expand All @@ -56,6 +59,7 @@ module.exports = {
embroider,
lang: options.lang,
ciProvider: options.ciProvider,
typescript: options.typescript,
};
},

Expand All @@ -82,4 +86,10 @@ module.exports = {
this.ui.writeLine('');
this.ui.writeLine(prependEmoji('✨', `Creating a new Ember app in ${chalk.yellow(process.cwd())}:`));
},

async afterInstall(options) {
if (options.typescript) {
await this.addAddonToProject({ name: 'ember-cli-typescript', blueprintOptions: options });
}
},
};
1 change: 1 addition & 0 deletions lib/commands/addon.js
Expand Up @@ -26,6 +26,7 @@ module.exports = NewCommand.extend({
default: 'github',
description: 'Installs the default CI blueprint. Either Travis or Github Actions is supported.',
},
{ name: 'typescript', type: Boolean, default: false, description: 'Set up the addon to use TypeScript' },
],

anonymousOptions: ['<addon-name>'],
Expand Down
1 change: 1 addition & 0 deletions lib/commands/init.js
Expand Up @@ -42,6 +42,7 @@ module.exports = Command.extend({
default: 'github',
description: 'Installs the default CI blueprint. Either Travis or Github Actions is supported.',
},
{ name: 'typescript', type: Boolean, default: false, description: 'Set up the app to use TypeScript' },
],

anonymousOptions: ['<glob-pattern>'],
Expand Down
1 change: 1 addition & 0 deletions lib/commands/new.js
Expand Up @@ -47,6 +47,7 @@ module.exports = Command.extend({
aliases: ['i'],
description: 'Create a new Ember app/addon in an interactive way.',
},
{ name: 'typescript', type: Boolean, default: false, description: 'Set up the app to use TypeScript' },
],

anonymousOptions: ['<app-name>'],
Expand Down
25 changes: 24 additions & 1 deletion lib/models/blueprint.js
Expand Up @@ -490,7 +490,7 @@ let Blueprint = CoreObject.extend({
// it's possible for people to set `{typescript: true}` in their `.ember-cli` file, which would
// then erroneously trigger this message every time they generate a JS blueprint even though
// they didn't pass the flag.
if (options.typescript === true) {
if (options.typescript === true && isJavaScriptFile(fileInfo.outputPath)) {
this.ui.writeLine(
chalk.yellow(
"You passed the '--typescript' flag but there is no TypeScript blueprint available. " +
Expand Down Expand Up @@ -1319,6 +1319,25 @@ let Blueprint = CoreObject.extend({
let installText = packages.length > 1 ? 'install addons' : 'install addon';
this._writeStatusToUI(chalk.green, installText, taskOptions['packages'].join(', '));

if (!this.project.isEmberCLIProject()) {
// our blueprint ran *outside* an ember-cli project. So the only way this
// makes sense if the blueprint generated a new project, which we're now
// ready to add some addons into. So we need to switch from "outside" mode
// to "inside" by reinitializing the Project.
//
// One might think the cache clear is unnecessary, because in theory the
// caches should be shareable, but in practice the new Project we create
// below will have the same root dir as the NullProject and that makes bad
// things happen.
this.project.packageInfoCache._clear();
const Project = require('../../lib/models/project');
this.project = taskOptions.blueprintOptions.project = Project.closestSync(
this.project.root,
this.project.ui,
this.project.cli
);
}

return this.taskFor('addon-install').run(taskOptions);
},

Expand Down Expand Up @@ -1811,4 +1830,8 @@ function isTypeScriptFile(filePath) {
return path.extname(filePath) === '.ts';
}

function isJavaScriptFile(filePath) {
return path.extname(filePath) === '.js';
}

module.exports = Blueprint;
3 changes: 2 additions & 1 deletion lib/models/project.js
Expand Up @@ -311,7 +311,7 @@ class Project {
if (fs.existsSync(`${targetsPath}.js`)) {
this._targets = this.require(targetsPath);
} else {
this._targets = require('../../blueprints/app/files/config/targets');
this._targets = { browsers: ['last 1 Chrome versions', 'last 1 Firefox versions', 'last 1 Safari versions'] };
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When requiring ../../blueprints/app/files/config/targets, node would for some reason also try to read from ../../blueprints/app/files/package.json, which is a template file, not a real one.

Previously this caused no issues, as the only parts with "template tags" (or whatever you call these <% %>) was within .name here. But now we have more conditionals in place, which make this package.json template invalid JSON (in the raw format), so node would fail parsing it.

As targets.js is a JS file, I also couldn't just simply read it without making node interpret it (like with something like fs.readJSONSync()), so the only fix I could come up with is to unfortunately just duplicate the exported value here. There is a test covering the output of this, so hopefully the chance of this and the actual targets file diverging is very low! 🤔

}
return this._targets;
}
Expand Down Expand Up @@ -496,6 +496,7 @@ class Project {
return;
}
this._addonsInitialized = true;
this._didDiscoverAddons = false;
simonihmig marked this conversation as resolved.
Show resolved Hide resolved

logger.info('initializeAddons for: %s', this.name());

Expand Down
6 changes: 4 additions & 2 deletions tests/acceptance/init-test.js
Expand Up @@ -47,10 +47,12 @@ describe('Acceptance: ember init', function () {
process.chdir(root);
});

function confirmBlueprinted() {
function confirmBlueprinted(typescript = false) {
let blueprintPath = path.join(root, 'blueprints', 'app', 'files');
// ignore .travis.yml
let expected = walkSync(blueprintPath, { ignore: ['.travis.yml'] }).sort();
let expected = walkSync(blueprintPath, { ignore: ['.travis.yml'] })
.map((name) => (typescript ? name : name.replace(/\.ts$/, '.js')))
.sort();
let actual = walkSync('.').sort();

forEach(Blueprint.renamedFiles, function (destFile, srcFile) {
Expand Down