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

[legacy-framework] feat(cli): use custom template format instead of EJS #281

Merged
merged 8 commits into from
Apr 28, 2020

Conversation

aem
Copy link
Collaborator

@aem aem commented Apr 26, 2020

Type: Feature

Closes: blitz-js/legacy-framework#823

What are the changes and their implications? ⚙️

This removes our usages of the .ejs templating language and replaces it with a custom templating system. Since we don't actually use any templating functionality other than pure variable replacement, we can move from mem-fs-copy's copyTpl function to the raw copy function which lets us supply a custom processing function. In our case, we just do global string replaces on any code that matches the __variable__ pattern with a variable that exists in the template.

As an extension, we had to centralize the generation logic to ensure that each generator doesn't have to re-implement this logic themselves. This drastically simplifies each Generator subclass which now just needs to supply a template name, a subdirectory within the project where its files should be placed, and a function that will return the values to pass to the template. I was initially worried that this API would be a bit too abstract, but I've come to like the fact that creating a new generator is now more about writing up the template and less about writing generation code. There's of course an escape hatch which is just to override the async write function.

Checklist

  • Tests added for changes
  • Any added terminal logging uses packages/server/src/log.ts

Breaking change: No

Other information

@aem aem requested a review from flybayer April 26, 2020 20:41
@flybayer
Copy link
Member

This is looking much better!!

  • .gitignore still isn't working right (I verified I pulled your latest code)
CREATE    db/schema.prisma
CREATE    gitignore
CREATE    integrations/.keep
  • Something in here is causing a significant hang between "Dependencies successfully installed." and "Your new Blitz app is ready! Next steps:" during blitz new

@flybayer flybayer marked this pull request as draft April 27, 2020 11:13
@aem
Copy link
Collaborator Author

aem commented Apr 27, 2020

@flybayer I'm guessing the delay is because the gitignore isn't getting copied, the CLI is probably trying to do some processing to the whole tree, some of which is now things like node_modules if the gitignore is malformed

@aem aem force-pushed the adam/remove-ejs branch from fcc81d4 to 96fb314 Compare April 27, 2020 15:03
@aem aem marked this pull request as ready for review April 27, 2020 15:09
@flybayer
Copy link
Member

Ok, delay is fixed and .gitignore is generated, but the log for gitignore is wrong:

CREATE    db/schema.prisma
CREATE    gitignore
CREATE    integrations/.keep

Missing the dot

@aem aem force-pushed the adam/remove-ejs branch from 96fb314 to 9373066 Compare April 28, 2020 13:58
@aem aem requested a review from merelinguist as a code owner April 28, 2020 13:58
@aem
Copy link
Collaborator Author

aem commented Apr 28, 2020

@flybayer 🤦 i fixed that but had the commit stashed while resolving some conflicts. pushing it up now

@merelinguist
Copy link

Okay I might be wrong but I think @Zeko369’s recent pull broke the ability to create new projects!

$ /Users/dylan/dev/blitz/node_modules/.bin/blitz new aem-test
You are using alpha software - if you have any problems, please open an issue here:
  https://github.com/blitz-js/blitz/issues/new/choose

You are not inside a Blitz project, so this command won't work.
You can create a new app with blitz new myapp or see help with 
 ›   Error: Not in correct folder
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@aem
Copy link
Collaborator Author

aem commented Apr 28, 2020

@merelinguist you're definitely correct. I'm going to open up a revert PR for that

@merelinguist
Copy link

Thanks!

@merelinguist
Copy link

Otherwise this all looks great, by the way 👍

@merelinguist
Copy link

@flybayer Looks grand to me, will let you merge.

@aem aem changed the title feat(cli): use custom template format feat(cli): use custom template format instead of EJS Apr 28, 2020
@aem aem merged commit 3bcc1de into canary Apr 28, 2020
@flybayer flybayer deleted the adam/remove-ejs branch November 13, 2020 00:22
@itsdillon itsdillon changed the title feat(cli): use custom template format instead of EJS [legacy-framework] feat(cli): use custom template format instead of EJS Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider changing from ejs templates to plain string replace
3 participants