[V2] Fix Circular Dependency Warnings

[bendvc]

Description

Lerna was spitting out circular dependency warning for almost every package (see image attached). This was because the runtime was including the cli in its devDependencies for use in a single test.

This PR removes that dependency and updates the test. The test is less concrete than before, but I think it's good enough for this case.

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

• (change1)

How to Test-Drive This PR

• (step1)

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)

[alexvuong]

We need this package though. We are using pwa-kit-cli for local dev. See here

[wjhsf]

The linked file (express.js) is the only place that pwa-kit-runtime uses pwa-kit-cli. The imported file (build-dev-server.js) is only used by express.js. To resolve the circular dependency, I think it makes sense to just move build-dev-server.js into pwa-kit-runtime, as it's not used by any other packages, not even by pwa-kit-cli.

[bendvc]

@alexvuong It actually isn't being used there. The usage on the line that you called out is covered by the peer dependency here

[bendvc]

@wjhsf I had similar thoughts. This would be more of an architectural change to @olibrook work, it would also go beyond fixing the circular dependency as it's not required to fix this issue. He might see this comment and weigh in.

[olibrook]

@alexvuong We do not need it – if we did, it wouldn't even have been in devDependencies before. That's why the code does the dynamic eval('require') import.

@bendvc Your suggestion looks good to me if you would like to silence the warnings. And the test strategy looks fine to me, too.

[alexvuong]

@bendvc got it. What @wjhsf said makes sense to me too.

[bendvc]

@alexvuong @wjhsf I reckon that @olibrook 's intentions where to separate the dev server factory from the runtime package is it doesn't really have anything to do with the runtime per-say. You can imaging that the runtime isn't concerned with how your application is developed, only that can run projects that follow its API.

For example, you can create a project that doesn't use webpack at all, and therefore doesn't use webpack-dever-server, so it doesn't make sense that we have that code in the runtime. Previously the cli package was called pwa-kit-build, and it sounded more like a package that might have a dev server in it, but now that it's called pwa-kit-cli it seems a little less likely to be a package that contains a dev sever factory. I think that might be the source of the code smell.

That being said, this is outside of the scope of this ticket I think. The circular dep only existed via a dev dependency, which we have removed. So I think we are good to go.

[alexvuong]

LGTM