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

Blueprint removes <title> from app/index.html upon install for FastBoot #177

Merged
merged 2 commits into from Sep 23, 2020

Conversation

raido
Copy link
Contributor

@raido raido commented Sep 23, 2020

Make blueprint remove title element from index.html if ember-cli-fastboot is present.

Closes #174.

}
);

const titleMatches = contents.match(/<title>(.*)<\/title>/i);
Copy link
Contributor Author

@raido raido Sep 23, 2020

Choose a reason for hiding this comment

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

While this works, maybe there are some other well formed ways to do edits like this from blueprints?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not aware of any, but @rwjblue might be more familiar.

Copy link
Member

Choose a reason for hiding this comment

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

In theory, we could use ember-template-recast here but that has issues with <DOCTYPE.

I think (since <title> doesn't allow nesting, and the location is stable/knowable) that this is fine...

@raido
Copy link
Contributor Author

raido commented Sep 23, 2020

#178 - this should fix the private router access caused linter failure.

@raido raido force-pushed the remove-title-element-upon-install branch from 0561c2b to 4aa0984 Compare September 23, 2020 20:16
@raido
Copy link
Contributor Author

raido commented Sep 23, 2020

It is rebased and CI should be fine now.

Should we try to add some tests for updated blueprint with https://github.com/ember-cli/ember-cli-blueprint-test-helpers or not worth it for now?

@knownasilya
Copy link
Contributor

Probably not worth it for now, if we have to revise this with something a bit more complicated then can revisit adding tests.

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.

Blueprint should remove <title> tag from app/index.html if Fastboot is present
3 participants