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

Consider changing from ejs templates to plain string replace #823

Closed
flybayer opened this issue Apr 25, 2020 · 2 comments · Fixed by blitz-js/blitz#281
Closed

Consider changing from ejs templates to plain string replace #823

flybayer opened this issue Apr 25, 2020 · 2 comments · Fixed by blitz-js/blitz#281

Comments

@flybayer
Copy link
Member

flybayer commented Apr 25, 2020

What do you want and why do you want it?.

Working on the ejs templates for blitz generate (which has syntax like <div<%= name %></div>) is currently a nightmare because syntax TS can't parse it, prettier can't parse it, eslint can't parse it etc.

This means you have to actually run the generator each time you make a change to see it's correct or not. And you have to manually format your code.

I think we should consider just doing plain string replaces instead.

Look how difficult this is to read and write:

import create<%= ModelName %> from 'app/<%= modelNames %>/mutations/create<%= ModelName %>'

This is SO much easier to read and write:

import createModelName from 'app/modelNames/mutations/createModelName`

Other motivations:

  • We will be adding template ejection for users to customize. So this affects all blitz users, not just blitz contributors.
  • We can run tests directly against the templates
  • This is also nicer for the template file names: createModelName.ts -> createProduct.ts

/cc @aem

@aem
Copy link
Contributor

aem commented Apr 26, 2020

I didn't realize this was a discussion rather than a ready-to-implement issue. I personally see very little downside here but lots of upside. Owning our own templating system increases flexibility, it means we own the full pipeline rather than being tied to an external system that can break out from under us, and the guaranteed cross-system compatibility is nice. The implementation is also a nice forcing function for centralizing the generator logic in the superclass since all of the generator code is functionally the same.

@aem
Copy link
Contributor

aem commented Apr 28, 2020

This seems uncontroversial, relabeling as ready for dev since the PR seems to be acceptable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants