Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement RemoteView #2284

Merged
merged 43 commits into from Mar 1, 2023
Merged

Conversation

sgb-io
Copy link
Contributor

@sgb-io sgb-io commented Feb 17, 2023

Implement the RemoteView package

Extracts the <RemoteView /> component from the Modular Microfrontends demo into a new library @modular-scripts/remote-view.

View the new docs at packages/remote-view/README.md.

What is different vs the existing demo?

  • New approach to error handling. More error cases handled vs the demo. See docs for details.
  • Loading state can be customised - see docs for details
  • Code broken up into smaller pieces
  • Unit tests plus puppeteer-based integration tests
  • Added docs
  • Added a demos package called remote-view-demos, which acts as a showcase and contains useful examples for consumers

What is the contents?

  • The new remote-view package
  • The new remote-view-demos package - storybook-like showcase of various examples of using <RemoteView />
  • New fixtures: 4 example ESM Views
  • New fixtures: remote-view-fake-cdn contains the 4 example ESM Views, pre-built, plus react/react-dom as they would be exposed on esm.sh. This allows us to fully test <RemoteView /> without a dependency on esm.sh (or other esm CDN). Unfortunately it's 12k lines tracked in the repo, but it's all contained within __fixtures__.

TODO

  • Loading component prop, with a default
  • Error handling approach
  • Installation bit on docs (mention the package name so that it can be copy/pasted)
  • Coverage - iframe fallback, error cases, loading state
  • Debug/fix broken ubuntu tests
  • Document what's different vs the existing demo in the PR
  • Notes on security
  • Permanently add RemoteView demo esm-view package
  • Clarify / test the seemingly-superfluous setState call
  • Handle non-RemoteViewError errors appropriately
  • Add changeset
  • Consider the benefits / trade-offs from loading views in the provider instead of the component, get consensus from team
  • Make changes to shift data fetching into the provider for max efficiency
  • Docs: Update docs after new error boundary approach
  • Docs: Update contributing docs for remote-view-fake-cdn. Maybe move this into a readme that is local to the fixture. Also, document how to use the component in local dev via another esm-view
  • Docs: Add docs for this to the public docs area - under Components?
  • Verdaccio e2e test. Ensure that a project consuming the components can be built and served and still work. Test this both with webpack but also esbuild.
  • Update docs to make esm-view and React constraints on the host very clear
  • Verify the GitHub publishing workflow for the new package

@changeset-bot
Copy link

changeset-bot bot commented Feb 17, 2023

🦋 Changeset detected

Latest commit: dda5fae

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@modular-scripts/modular-types Minor
@modular-scripts/remote-view Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR


node.insertAdjacentHTML(
'beforeend',
`<link rel='stylesheet' href='${url}' />`,

Check warning

Code scanning / CodeQL

Unsafe HTML constructed from library input

This HTML construction which depends on [library input](1) might later allow [cross-site scripting](2).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding this, we've added a security note to the docs, plus the code.

does need to be used responsibly, and developers should ensure they only load ESM Views from trusted sources

@cristiano-belloni cristiano-belloni changed the base branch from main to feature/v4.1 February 21, 2023 11:56
@cristiano-belloni
Copy link
Contributor

@sgb-io I moved the base to feature/v4.1

@sgb-io sgb-io marked this pull request as ready for review February 27, 2023 15:14
packages/remote-view/README.md Outdated Show resolved Hide resolved
packages/remote-view/README.md Outdated Show resolved Hide resolved
@cristiano-belloni
Copy link
Contributor

I think you need to build RemoteView in the root package.json too.

@sgb-io sgb-io merged commit a4e2f21 into jpmorganchase:feature/v4.1 Mar 1, 2023
@sgb-io sgb-io deleted the feat/remote-view branch March 1, 2023 14:23
@github-actions github-actions bot mentioned this pull request Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants