-
Notifications
You must be signed in to change notification settings - Fork 4
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
Created ember-codemod-remove-ember-css-modules (Part 2) #11
Created ember-codemod-remove-ember-css-modules (Part 2) #11
Conversation
✅ Deploy Preview for embroider-css-modules ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
export default class <%= entity.classifiedName %>Controller extends Controller { | ||
<%= options.__styles__ %> = <%= options.__styles__ %>; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Glint projects, when the route template refers to this.model
or @model
, we need to define its type in the controller.
Though the codemod could add a placeholder line,
/* app/controllers/products.ts */
import Controller from '@ember/controller';
import styles from './products.css';
export default class ProductsController extends Controller {
/* TODO: Define the type of model */
declare model;
styles = styles;
}
that placeholder line is unnecessary when the route template doesn't refer to model
(this may cause an extra type or linting error).
In short, the codemod has to do extra computations to figure out when exactly model
needs to be declared and what type it should take. Having extra code is more likely to cause bugs and increase the maintenance cost.
For simplicity, in a TypeScript project (regardless of Glint), all new controllers will not include the declare model
. The end-developer is expected to fix the type errors.
const blueprintFilePaths = project.hasTypeScript | ||
? ['ember-cli/component/typescript.ts'] | ||
: ['ember-cli/component/javascript.js']; | ||
|
||
const fileMapping = new Map( | ||
blueprintFilePaths.map((blueprintFilePath) => { | ||
const filePath = getFilePath(entityName, options); | ||
|
||
const blueprintFile = readFileSync( | ||
join(blueprintRoot, blueprintFilePath), | ||
'utf8' | ||
); | ||
|
||
const file = processTemplate(blueprintFile, { | ||
entity: parseEntityName(entityName), | ||
options, | ||
}); | ||
|
||
return [filePath, file]; | ||
}) | ||
); | ||
|
||
createFiles(fileMapping, options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I structured the code so that there's a high similarity to ember-codemod-v1-to-v2
and I can potentially extract a function.
function updateClass(entityName, options) { | ||
// ... | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be implemented later.
const blueprintFilePaths = project.hasTypeScript | ||
? ['ember-cli/controller/typescript.ts'] | ||
: ['ember-cli/controller/javascript.js']; | ||
|
||
const fileMapping = new Map( | ||
blueprintFilePaths.map((blueprintFilePath) => { | ||
const filePath = getFilePath(entityName, options); | ||
|
||
const blueprintFile = readFileSync( | ||
join(blueprintRoot, blueprintFilePath), | ||
'utf8' | ||
); | ||
|
||
const file = processTemplate(blueprintFile, { | ||
entity: parseEntityName(entityName), | ||
options, | ||
}); | ||
|
||
return [filePath, file]; | ||
}) | ||
); | ||
|
||
createFiles(fileMapping, options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I structured the code so that there's a high similarity to ember-codemod-v1-to-v2
and I can potentially extract a function.
function updateClass(entityName, options) { | ||
// ... | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be implemented later.
@@ -7,7 +7,7 @@ const __dirname = dirname(__filename); | |||
function getBlueprintRoot() { | |||
const srcDirectory = join(__dirname, '../..'); | |||
|
|||
return join(srcDirectory, 'blueprints/ember-app'); | |||
return join(srcDirectory, 'blueprints'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The prior definition came from ember-codemod-v1-to-v2
. I realized that it wasn't general enough and will need port the fix.
interface <%= entity.classifiedName %>Signature { | ||
// The arguments accepted by the component | ||
Args: {}; | ||
// Any blocks yielded by the component | ||
Blocks: { | ||
default: []; | ||
}; | ||
// The element to which `...attributes` is applied in the component template | ||
Element: null; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment, ember-cli
creates the interface for TS projects only when we pass an environment variable:
EMBER_TYPESCRIPT_BLUEPRINTS=true ember g component navigation-menu
<% if (options.project.hasGlint) { %> | ||
declare module '@glint/environment-ember-loose/registry' { | ||
export default interface Registry { | ||
'<%= entity.doubleColonizedName %>': typeof <%= entity.classifiedName %>Component; | ||
'<%= entity.name %>': typeof <%= entity.classifiedName %>Component; | ||
} | ||
} | ||
<% } %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To my knowledge, Glint does not provide yet a blueprint for components. This codemod may be the first to do so. :)
Description
It's difficult to manually migrate a production project away from
ember-css-modules
.One reason is the need to import
styles
in every component and route class. Then, thatstyles
needs to be added to the class so that the template has access. (Here, we assume that<template>
-tag components don't exist yet.)I updated the codemod so that it handles the migration fast and accurately. In particular, the pull request demonstrates how we can use blueprint files to create a class when it doesn't exist yet. This scenario happens when:
local-class
already) and doesn't have a backing class.local-class
already) and doesn't have a backing class.In a separate pull request, I will update the codemod so that the backing class is edited when it exists already (a much harder problem).
References
::
(4)