#1617 - Fetch store name from the GraphQL server at compile time

[vasilii-b]

Description

With the changes from this PR, the STORE_NAME is not hardcoded anymore from the webpack's DefinePlugin

pwa-studio/packages/venia-concept/webpack.config.js

Line 66 in 70cf1e0

STORE_NAME: JSON.stringify('Venia'),

As suggested in #1617, if STORE_VIEW_CODE environment variable is used, the store_name value from the corresponding available store is used. If not, use the default value returned by the getStoreConfigData query is used for the STORE_NAME.

Related Issue

Closes #1617.

Acceptance

Verification Stakeholders

@revanth0212

Specification

Changes to the venia-concept and pwa-buildback packages must be reviewed.

Verification Steps

Setup and run the webserver.

1. Go to the homepage.
2. View page's source code.
3. Find the title tag
4. Make sure the title tag value contains the store name as specified by the STORE_VIEW_CODE (or the default store).

Screenshots / Screen Captures (if appropriate)

Checklist

• I have added tests to cover my changes, if necessary.
• I have added translations for new strings, if necessary.
• I have updated the documentation accordingly, if necessary.

[PWAStudioBot]

	Messages
📖	DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖	Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

Generated by 🚫 dangerJS against b2b724f

[sirugh]

Thanks for the contribution! I had a small bit of feedback and please run prettier :)

[revanth0212]

Thanks for your contribution. Please take care of the change @sirugh requested and also handle the suggestion I have provided below. With those handled, this PR is good to go.

[sirugh]

A few more thoughts I had last second.

[sirugh]

@vasilii-b I just pushed a commit with an idea that I had, not something we're positive that we want just yet. I will work with the internal team to make a decision about how we want to proceed.

[brendanfalkowski]

Two cents — we went back/forth on this. Some things like our manifest + static icons have to be overwritten when our Docker containers are built for our multi-site frontends. But the title ended up being simple enough to hardcode a conditional with a reasonable default (for us).

I know that's not a config-only solution, but it saves an extra request at render, so we've ended up just hardcoding a lot of Magento config in frontend code that's only set once and never changed. We've started doing this as a rule now (fetching config should never delay render) for performance, but I know it only solves this at the implementation-level not the framework.

Still curious to see what you come up with @sirugh

[sirugh]

Still curious to see what you come up with @sirugh

@brendanfalkowski I pushed my approach which hopes to solve two problems:

1. Build time store config is not valuable since it is store specific (indicated by store code in the header).
2. Many components pass STORE_NAME.

I wanted to prevent developers from needing to make a store config query in every component, so the reasonable take was to do a single fetch somewhere. Ended up making a new Title component that just appends the fetched name.

[sirugh]

Is this OK? The app renders the title, not sure if we need the service worker template to do this. We wouldn't be able to ensure this is valid anyways.

[dpatil-magento]

@sirugh Browser title looks good but offline mode on Category and Search is broken.

Steps -

1. Visit Home, category, product and load some search results.
2. Now go offline mode and try to load above

Issue - Category and search does not load.

[sirugh]

Sort of figured it out. The category page uses a conditional to render an ErrorView based on whether an error is returned. For some reason there is an error being returned now in the category query, so it renders even though there is cached data to render.

Edit: I was right, but the issue stemmed from not having merged in fixes for this exact but that were in develop. The merge contains the correct code :)

[dpatil-magento]

QA Approved.