Site name that contains empty spaces is incorrectly displayed in browser tab

[regisb]

This is how a site name that includes white spaces appears in my browser tab:

This bug is almost certainly caused by this recent change: https://github.com/overhangio/tutor-mfe/pull/59

@ghassanmas I should have tested your change more thoroughly. Can you please suggest a fix?

[ghassanmas]

@regisb, Did you build the mfe image on a MAC/Zsh OS?

[regisb]

Nope, a regular x64 Linux.

[ghassanmas]

I couldn’t reproduce, Can you provide more details; Were you running which version of tutor, on which branch nutmeg or nightly? On dev or local mode and lastly which version/tag of the MFE

On 3 Oct 2022, at 17:24, Régis Behmo @.***> wrote:

Nope, a regular x64 Linux.

— Reply to this email directly, view it on GitHub https://github.com/overhangio/tutor-mfe/issues/61#issuecomment-1265522472, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD42PH4HG5RKEBWM642Y26TWBLUBPANCNFSM6AAAAAAQ3TDD7E. You are receiving this because you were mentioned.

[regisb]

You caught me! I'm usually the one asking for this kind of details :-p

$ tutor --version
tutor, version 14.0.5
$ tutor plugins list | grep mfe
mfe                             14.0.1
$ tutor dev start -d

The issue is observed in dev mode, in particular in the account MFE: http://apps.local.overhang.io:1997/account

Indeed, I expect that the issue will not occur in "local" mode because of an implementation quirk which I just realised: the values in mfe/build/mfe/env/ are parsed by bash in production:

RUN bash -c "set -a && source /openedx/env/production && source /openedx/env/production.override && npm run build"

But in development they are parsed by docker-compose:

env_files:
    - ../plugins/mfe/build/mfe/env/production
    - ../plugins/mfe/build/mfe/env/development

The latter is the reason why we removed single quotes in the env files: https://github.com/overhangio/tutor-mfe/pull/56

The fact that these files need to be parsed by two different tools (bash and docker-compose) is a source of confusion. I think that we should resolve that by making sure that these files are parsed only by bash. This way, we can add "..." quotes to the env files and stop bothering whether docker-compose will parse them correctly.

I'll open a corresponding PR.

[ghassanmas]

That’s a good summary!, if I can suggest another possibility;

Why not bind production to .env and development to .env.development doing that would allow to change configuration without relying on env variables. Fronted-build is already using this https://github.com/mrsteele/dotenv-webpack hence ref 1 https://github.com/openedx/frontend-build/blob/95e8a8c24095f1bbe20d48c965cc7bfb75cbd464/config/webpack.dev.config.js#L20-L22/dev and ref 2/prod https://github.com/openedx/frontend-build/blob/95e8a8c24095f1bbe20d48c965cc7bfb75cbd464/config/webpack.prod.config.js#L194-L197 this web pack plugin I guess would transform variables from a file and then inject them in process.env module

[regisb]

Why not bind production to .env and development to .env.development doing that would allow to change configuration without relying on env variables.

Hmmm... that's possible, but:

1. In development, are values from .env also loaded? If yes, in which order? If not then we need to list all variables in .env.development: currently this is not the case, because development values only override production values.
2. Do environment variables override values in .env/.env.development? We need to make it possible for some MFEs to override certain values, as we currently do with *_MFE_APP["env"]["production"].

Generally speaking, I wouldn't touch webpack tools with a ten foot pole, but if we can simplify the loading and overriding of settings by better leveraging webpack-env, then I'm all for it.

@arbrandes what are your thoughts on this?

[ghassanmas]

1.In development, are values from .env also loaded? If yes, in which order? If not then we need to list all variables in .env.development: currently this is not the case, because development values only override production values.

I don't think so, the relevant code I found about this frontend-build, is that you can .env.private file which would override, .env.development: ref code ref doc

2. Do environment variables override values in .env/.env.development? We need to make it possible for some MFEs to override certain values, as we currently do with *_MFE_APP["env"]["production"].

Yes I tested this the other day, not through *_MFE_APP["env"]["production"] per say, I injected env varaiable before the bash command, i.e. NAME=VALUE npm run build that ovrirde the value in .env file.

[arbrandes]

@regisb, @ghassanmas, just an ACK, for now: I'll take a look at the PR shortly.

[arbrandes]

Why not bind production to .env and development to .env.development

I'd rather leave those with each MFE's default values, and instead use the process environment to override them. This is more in line with how Docker does things. (Principle of Least Surprise, and all.)

Which is what @regisb is doing in the PR which I'm about to approve.

[ghassanmas]

Aha make sense, by the way are you going to keep env/* files after #69, my understanding is that we are going to remove, them right?, and then the we somehow are going just set only needed variables for build, i.e. the first point in #86, also other you mention in a comment at #69

[arbrandes]

@ghassanmas, we can't remove env files as part of #69: there still needs a way for Tutor to override the default values non-dynamically per MFE, including the all-important config API URL.

We could do it via patch to the Django settings files, but that only gets fetched after initialization.