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

Replace vite module server with wmr/custom module server #115

Merged
merged 18 commits into from
Jun 25, 2021
Merged

Conversation

calebeby
Copy link
Member

@calebeby calebeby commented Jun 22, 2021

Reviewing

It might be easiest to review in:
https://github1s.com/cloudfour/pleasantest/pull/115

To review the changes to src/module-server/rollup-plugin-container.ts, use this diff which shows the differences from WMR's version of that file.

Remaining issues:

Source Maps

Since WMR does not support source maps, Pleasantest's code to fix the stack traces from in-browser errors does not work. So I commented out the source-map-dependent code in Pleasantest for now. I will follow up with a PR to add source map support.

For now, errors from runJS/runJS-imported code look like this 🤮:

Screen Shot 2021-06-22 at 12 39 56 PM

I added .skip for the tests for this functionality, so in the follow-up PR I'll re-enable those tests.

Configurability of module server

Right now the configuration is not exposed. This is a pretty easy feature to add but I figured I'd leave it for a follow-up PR since this PR is already really big, only to get the tests passing.

Proposed API:

import { withBrowser, configure } from 'pleasantest'

// Global default configuration
configure({
  plugins: [
    /* some rollup plugins here */
  ],
  esbuild: {
    /* some option here */
  },
  // ...
})

test(
  'asdf',
  withBrowser({
    // overrides to global configuration (along with other options to withBrowser like device emulation)
  }, async () => {
    // test here
  }),
)

@calebeby calebeby marked this pull request as ready for review June 22, 2021 22:02
Copy link
Member

@gerardo-rodriguez gerardo-rodriguez left a comment

Choose a reason for hiding this comment

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

I left a few questions, otherwise, looks good to me. Quite the PR! 😅

@calebeby calebeby merged commit b4eb08d into main Jun 25, 2021
@calebeby calebeby deleted the module-server branch June 25, 2021 18:37
@github-actions github-actions bot mentioned this pull request Jun 25, 2021
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.

2 participants