Generator creates a project with a singe-locale #325

adamraya · 2022-01-18T00:48:41Z

GUS: https://gus.lightning.force.com/lightning/r/ADM_Work__c/a07EE00000TshIdYAJ/view

Description

The PWA Kit now supports multiple locales, but we want the generator to create a simplified project configured to use only a single locale. Users will be able to add multiple locales supported by PWA KIt by following the provided documentation.

This PR updates the generator script create-mobify-app.js to generate a project with the en-US locale and the RefArch siteId .

Since we'll work only with one locale, the URL configuration locale type for the generated project it's set to 'none' in the pwa-kit.config.js file. This removes the locale shortcode from the URL.

A lot of tests have been updated since they were using the hardcoded locale en-GB and thus all these tests were failing on a generated project using the single locale en-US.
These tests now use the locale defined as defaultLocale in package.json and follow the configuration set in the URL configuration locale type on the pwa-kit.config.js file to determine if the locale shortcode should be included in the URL pathname.

Note: I decided to copy the content of the en-GB translations to the en-US translation since we need the strings for the default language to render the app.
Once we have the real en-US translation, we'll need to update the content of the en-US file.

Types of Changes

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Documentation update
Breaking change (could cause existing functionality to not work as expected)
Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

Removing a public function or component or prop

Adding a required argument to a function

Changing the data type of a function parameter or return value

Adding a new peer dependency to package.json

Changes

Updated generator to create a project with the single locale en-US and the locale type URL configuration set to none.
Fixed a lot of tests with the hardcoded locale en-GB.

How to Test-Drive This PR

Checkout this branch generator-single-site-singe-locale.
Run npm ci.
Generate a project locally by running the create-mobify-app.js script:

GENERATOR_PRESET=test-project node packages/pwa-kit-create-app/scripts/create-mobify-app-dev.js --outputDir /tmp/project

Verify

Start the generated project npm start.
Navigate through the app verifying that locale en-US is used, the data is from theRefArch siteId and the locale shortcode is not being displayed in the URL.
Confirm the l10n settings in the generated project package.json look like this:

"l10n": {
    "supportedCurrencies": [
      "USD"
    ],
    "defaultCurrency": "USD",
    "supportedLocales": [
      {
        "id": "en-US",
        "preferredCurrency": "USD"
      }
    ],
    "defaultLocale": "en-US"
  }

Also confirm the URL configuration for the locale type in the pwa-kit.config.js file looks like this:

{
  "url": {
    "locale": "none"
  }
}

Run tests

All tests pass for the PWA: npm run test --prefix packages/pwa
The same tests pass for the generated project: npm run test

Checklists

General

Changes are covered by test cases
CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

There are no changes to UI

or...

Changes were tested with a Screen Reader (iOS VoiceOver or Android Talkback) and had no issues
Changes comply with WCAG 2.0 guidelines levels A and AA
Changes to common UI patterns and interactions comply with WAI-ARIA best practices

Localization

Changes include a UI text update in the Retail React App (which requires translation)

vcua-mobify · 2022-01-18T01:21:28Z

It looks like some of the CI tests are failing because they are hard coded to expect /en-GB as part of the url. We'll need to update the tests to be locale agnostic

adamraya · 2022-01-18T01:55:12Z

It looks like some of the CI tests are failing because they are hard coded to expect /en-GB as part of the url. We'll need to update the tests to be locale agnostic

Yeah, I saw that. I'll work on updating those tests tomorrow.

vmarta · 2022-01-19T20:30:03Z

@adamraya It looks like you're still working on this PR. Pls let me know when it's ready for review, thanks.

adamraya · 2022-01-20T00:24:58Z

@adamraya It looks like you're still working on this PR. Pls let me know when it's ready for review, thanks.

Yeah, sorry, we had a bunch of tests with the locale hardcoded that started to fail.

The tests are passing now. The PR is ready for review @vmarta.

vmarta · 2022-01-20T18:22:11Z

packages/pwa/app/utils/locale.test.js

-// Otherwise, our code would fall back to default and incorrectly pass the tests
-const supportedLocale = supportedLocales[1]
+
+const supportedLocale = supportedLocales[0]


🤔 hmm, this test file assumes that it's working with a multi-locale setup.

I see that we have updated the tests such that they would pass for both the pwa and the generated pwa project. But since the two have deviated from each other (one is a multi-locale site, while the other one is now a single locale), the tests can be more tricky to write and may not be clear to new developers.

How about we consider not running the tests for the generated project?

pwa-kit/.circleci/config.yml

Lines 243 to 244 in 0073095

- runtests:

cwd: /tmp/project

The generated project is our final most crucial product shipped to customers.
I think it is vital for us to run tests on the code that we hand to customers to make sure that the code they use as starting point works as expected.

The final transient dependencies installed in a generated project can be different from the set of dependencies installed in the PWA, they end up being different projects, thus in my opinion we still need to run tests on both.

You're right that writing tests that work for a single-locale and multi-locale setup can be harder to write.

Let me see what I can do on that particular test file to cover both single-locale and multi-locale scenarios in a simpler way.

I restored the original functionality on these tests and added a conditional with values for the specific single locale scenario.

vmarta · 2022-01-20T18:37:45Z

packages/pwa-kit-create-app/scripts/create-mobify-app.js

+const pkgSingleLocaleData = {
+    l10n: {
+        supportedCurrencies: ['USD'],
+        defaultCurrency: 'USD',
+        supportedLocales: [
+            {
+                id: 'en-US',
+                preferredCurrency: 'USD'
+            }
+        ],
+        defaultLocale: 'en-US'
+    }
+}
+
+const PWAKitConfigSingleLocaleData = {
+    url: {
+        locale: 'none'
+    }
+}


How about extracting these config-related objects into files in the folder 'packages/pwa-kit-create-app/assets/'? The ticket asks about creating files in that asset folder. It may be better to group all the files together in the same location.

Good call! I didn't know that we added an /assets folder to the generator and I missed that implementation detail.

Done, now the configuration objects are in files inside the assets folder.

sejal-salesforce · 2022-01-21T19:30:08Z

@adamraya The steps to testdrive PR confused me, so I want to confirm the behavior here. The default project = when no preset is given to npx pwa-kit-create-app. Is that right?

adamraya · 2022-01-21T19:40:04Z

https://gus.lightning.force.com/lightning/r/ADM_Work__c/a07EE00000TshIdYAJ/view

@sejal-salesforce Yes, with these changes, running the generator using npx pwa-kit-create-app will generate a project with a single locale.

However, the ticket doesn't refer to presets at all.
If we want to generate a project with a different locale configuration based on presets, that will be out of scope for this ticket and we'll need to address that work in a new ticket.

We can not test the changes using npx pwa-kit-create-app until we release the changes to NPM. The equivalent command to run the generator locally from the root of the monorepo is:

GENERATOR_PRESET=test-project node packages/pwa-kit-create-app/scripts/create-mobify-app-dev.js --outputDir /tmp/project

The command uses the test-project preset just to automate filling the generator questions with the default answers.

This PR doesn't change the questions/answers or the presets.

vmarta

Thanks for the latest changes. They look good. I also tested generating a project locally and running it. It's good too.

adamraya added 6 commits January 14, 2022 11:11

Single locale en-US and single site RefArch

2c788e5

Build en-US translations

68bffc4

Set pwa-kit config url locale to none

1adaba1

Update create-mobify-app script wip

58b5ea7

debug create-mobify-app script

c4f7f89

Use deepmerge to add single locale data

1db75c1

adamraya requested a review from a team as a code owner January 18, 2022 00:48

Revert test changes in the pwa

51c30e5

adamraya added the ready for review PR is ready to be reviewed label Jan 18, 2022

adamraya added 2 commits January 17, 2022 17:46

Set locale to none in PWAKit Config data & Fix tests

89b88be

Fix PWA Kit config json file path

1984967

adamraya added wip Work in process and removed ready for review PR is ready to be reviewed labels Jan 18, 2022

adamraya added 8 commits January 18, 2022 10:00

Fix test wip

bb319c9

More fixing test wip

fc7a109

Fix locale type in order test URLs

be721ca

Fix locale type in account tests URLs

8d73a8e

Fix more locale type in more URLs

fffdaba

lint

2ead22f

Refactor adding tests utils getPathname

782d5ab

lint

1c48384

adamraya added 2 commits January 19, 2022 15:58

Merge branch 'develop' into generator-single-site-singe-locale

e0387be

Update test use-auth-modal create an account test condition

4e9f7c6

adamraya added ready for review PR is ready to be reviewed and removed wip Work in process labels Jan 20, 2022

adamraya changed the title ~~Generator creates a project with a single-site and a singe-locale~~ Generator creates a project with a singe-locale Jan 20, 2022

Update index.test.js

0073095

vmarta reviewed Jan 20, 2022

View reviewed changes

adamraya added 2 commits January 21, 2022 10:30

Extract pwa-kit.config.json to assets folder

060e2cf

Extract l10n config to assets folder

cff1447

adamraya added 2 commits January 21, 2022 11:54

Update locale.test.js

ffbc008

Merge branch 'develop' into generator-single-site-singe-locale

27383bd

vmarta approved these changes Jan 25, 2022

View reviewed changes

adamraya and others added 3 commits January 25, 2022 11:50

Merge branch 'develop' into generator-single-site-singe-locale

3846abd

Merge branch 'develop' into generator-single-site-singe-locale

0b4a460

Update CHANGELOG.md

5a9fbae

adamraya merged commit a3834ea into develop Jan 25, 2022

vcua-mobify mentioned this pull request Jan 26, 2022

Generator with demo settings #322

Merged

12 tasks

adamraya deleted the generator-single-site-singe-locale branch January 29, 2022 00:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generator creates a project with a singe-locale #325

Generator creates a project with a singe-locale #325

adamraya commented Jan 18, 2022 •

edited

Loading

vcua-mobify commented Jan 18, 2022

adamraya commented Jan 18, 2022

vmarta commented Jan 19, 2022

adamraya commented Jan 20, 2022

vmarta Jan 20, 2022

vmarta Jan 20, 2022

adamraya Jan 21, 2022 •

edited

Loading

adamraya Jan 21, 2022

vmarta Jan 20, 2022

adamraya Jan 21, 2022 •

edited

Loading

sejal-salesforce commented Jan 21, 2022

adamraya commented Jan 21, 2022 •

edited

Loading

vmarta left a comment

Generator creates a project with a singe-locale #325

Generator creates a project with a singe-locale #325

Conversation

adamraya commented Jan 18, 2022 • edited Loading

Description

Types of Changes

Changes

How to Test-Drive This PR

Verify

Run tests

Checklists

General

Accessibility Compliance

Localization

vcua-mobify commented Jan 18, 2022

adamraya commented Jan 18, 2022

vmarta commented Jan 19, 2022

adamraya commented Jan 20, 2022

vmarta Jan 20, 2022

Choose a reason for hiding this comment

vmarta Jan 20, 2022

Choose a reason for hiding this comment

adamraya Jan 21, 2022 • edited Loading

Choose a reason for hiding this comment

adamraya Jan 21, 2022

Choose a reason for hiding this comment

vmarta Jan 20, 2022

Choose a reason for hiding this comment

adamraya Jan 21, 2022 • edited Loading

Choose a reason for hiding this comment

sejal-salesforce commented Jan 21, 2022

adamraya commented Jan 21, 2022 • edited Loading

vmarta left a comment

Choose a reason for hiding this comment

adamraya commented Jan 18, 2022 •

edited

Loading

adamraya Jan 21, 2022 •

edited

Loading

adamraya Jan 21, 2022 •

edited

Loading

adamraya commented Jan 21, 2022 •

edited

Loading