Skip to content
This repository was archived by the owner on May 23, 2025. It is now read-only.

Feature/un webpacked configuration#57

Open
orioltf wants to merge 7 commits intodevelopfrom
feature/un-webpacked-configuration
Open

Feature/un webpacked configuration#57
orioltf wants to merge 7 commits intodevelopfrom
feature/un-webpacked-configuration

Conversation

@orioltf
Copy link
Copy Markdown
Member

@orioltf orioltf commented Mar 17, 2017

Proposal to add JS configuration file(s) which are not processed by Webpack. This is useful for backend developers, for instance, to properly point the css file to be loaded by the fontLoader.

@marbor3
Copy link
Copy Markdown
Collaborator

marbor3 commented Apr 21, 2017

Hmm,

I see the problem - I don't remember where but we have had exactly the same issue (everywhere? :)

BUT!

I'm not sure if this approach will be usable for BE - we're storing their configuration in our repository - what if John has fonts in /john/fonts/ and Jane has her in /jane/assets/201705/fonts/, what if we need something else: Port numbers, database hook, credentials?

We need this configuration code - that for sure - but one thing - it's just few lines - do we need another request for this - it can be just inline script right? So maybe lets just create a partial with inline script - included in each layout that holds every possible option available to set (with description)

Or did I not understand the problem here? :)

# Conflicts:
#	source/layouts/layout.hbs
@orioltf
Copy link
Copy Markdown
Member Author

orioltf commented May 9, 2017

Hi @marbor3

I decided having a JS file because it can be easier to require dependencies in there if needed, though I agree with you that initially it should be "only some lines" and in this regard an inline script tag could maker it anyway. Additionally, some Editors/IDEs may have less support for editing JS code in an .hbs file. I could try implementing your suggestion anyway, of course, but I don't see that many features: BE can always decide to inline that script, of course

Regarding your first worry: this is a prototype, for COOP we already developed 2 projects for them without any issue. They understand that they have to recreate the configuration file for their needs, and they even have different ones for each co-worker, for tests systems or for production. A single parameter in each build system manages which file to load.

Regarding the amount of information available in the configuration files: our prototype may need more or less, and backend integration requires normally that we adapt a couple of things. Then we adapt them and if they need we to add configuration params I cannot see any issue.

Is this answering your questions? :)

@backflip
Copy link
Copy Markdown
Collaborator

backflip commented Jun 8, 2017

I like the switch from external files to a partial. However, I'd simplify this even more before merging into the default Estático setup:

  • Having a single partial options.hbs along the lines of
window.estatico = window.estatico || {};
window.estatico.options = {{options}};
  • Adding an options key to default.data.js, containing fontLoaderUrl and friends
  • Documenting an example where fontLoaderUrl would be customized based on require('gulp-util').env (right within default.data.js)

IMHO, this would reduce the complexity a lot (no conditions in layout files, no hard-coded config in handlebars files) without losing any flexibility. I would assume that most projects don't have an acceptance env, so I'd setup the default for them.

@backflip
Copy link
Copy Markdown
Collaborator

@orioltf, what is your opinion on simplifying this to adding an options key (see details above)?

@marbor3 marbor3 removed their request for review December 28, 2021 10:05
@marbor3 marbor3 removed their assignment Dec 28, 2021
@marbor3
Copy link
Copy Markdown
Collaborator

marbor3 commented Jan 17, 2022

Shall we close it @orioltf @backflip ?

@backflip
Copy link
Copy Markdown
Collaborator

@marbor3, @orioltf, I would rather set these things up in an environment like Storybook as I don't think extending Estatico makes sense anymore.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants