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

Simplify dev server #1026

Merged
merged 1 commit into from
May 16, 2023
Merged

Simplify dev server #1026

merged 1 commit into from
May 16, 2023

Conversation

acelaya
Copy link
Contributor

@acelaya acelaya commented May 15, 2023

This PR simplifies the script that runs the development express server. The changes are:

  • Remove mustache entirely. The only template we use is actually static html, so we don't need a templating system.
  • Expose the images folder as a static source, so that we can access source icon images.

Those two changes make the dev server closer to how the patterns docker container behaves.

With these changes, the templates folder might not make a lot of sense anymore. Maybe we can later consider having a public folder containing the index.html file and the images folder on it. So basically everything which is already static, does not need any building to be served, and we want to be accessible via HTTP.

public
├── images
│   └── icons
│       └── ...
└── index.html

Testing steps

  1. Check out this branch
  2. Run dev server: make dev
  3. Make sure the overall navigation works in http://localhost:4001
  4. Verify static images can also be accessed locally http://localhost:4001/images/icons/annotate.svg (any ther icon should also work).

@acelaya acelaya requested a review from lyzadanger May 15, 2023 15:39
app.get('/:path?', (req, res) => {
res.render('pattern-library');
res.sendFile(path.join(dirname, '../templates/index.html'));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From express docs:

How do I render plain HTML?
You don’t! There’s no need to “render” HTML with the res.render() function. If you have a specific file, use the res.sendFile() function

https://expressjs.com/en/starter/faq.html#how-do-i-render-plain-html

Copy link
Contributor

@lyzadanger lyzadanger left a comment

Choose a reason for hiding this comment

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

Thanks! I do agree that we can do further work to simplify the structure of the static assets.

Note that the images/icons directory is referenced by the generate-icons script and will need to be updated if those icons are moved.

@acelaya acelaya merged commit baa4a18 into main May 16, 2023
@acelaya acelaya deleted the simplify-dev-server branch May 16, 2023 06:59
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.

2 participants