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

Add Remix Example #99

Merged
merged 14 commits into from
Jun 12, 2022
Merged

Add Remix Example #99

merged 14 commits into from
Jun 12, 2022

Conversation

sonngdev
Copy link
Collaborator

@sonngdev sonngdev commented May 3, 2022

Issue: #80

Chores

  • Add Remix example
  • Link this example to website
  • Test with images to see if they works
  • Handle absolute import in Remix app
  • Investigate why after merging with main, Remix example stops working (probably because I forgot to rebuild the source)
  • Make Jest Preview work with different CSS supports from Remix. (likely to be handled in a different PR)

Notes

@netlify
Copy link

netlify bot commented May 3, 2022

Deploy Preview for jest-preview-library ready!

Name Link
🔨 Latest commit 9633fd7
🔍 Latest deploy log https://app.netlify.com/sites/jest-preview-library/deploys/62a6213d2041180008c1b5b7
😎 Deploy Preview https://deploy-preview-99--jest-preview-library.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@sonngdev sonngdev marked this pull request as draft May 3, 2022 14:29
@sonngdev sonngdev linked an issue May 3, 2022 that may be closed by this pull request
Copy link
Owner

@nvh95 nvh95 left a comment

Choose a reason for hiding this comment

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

Thank you. It's already amazing work. I left 2 comments when skimming your PR.

examples/remix/README.md Outdated Show resolved Hide resolved
examples/remix/README.md Outdated Show resolved Hide resolved
@sonngdev sonngdev marked this pull request as ready for review May 8, 2022 15:43
Copy link
Owner

@nvh95 nvh95 left a comment

Choose a reason for hiding this comment

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

I just leave some comments.
@thanhsonng Do you want to add a short blog post to announce that we support remix?

examples/remix/README.md Show resolved Hide resolved
Comment on lines 7 to 11
## Install Jest

You have to manually install Jest in your Remix app.
Copy link
Owner

Choose a reason for hiding this comment

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

I think the main concentrate of this guide is how to integrate Jest Preview to remix. However, they need to configure Jest to work with Remix first. I suggest you create a repository/ blog post on your own to guide others to setup Jest with Remix standalone, then refer to the link here.

In this guideline, just keep it as concise as possible.

Comment on lines 17 to 59
Install Babel

```bash
npm install --save-dev @babel/preset-env @babel/preset-react babel-jest
```

Config Babel inside `babel.config.js`

```bash
module.exports = {
presets: [
['@babel/preset-env', { targets: { node: 'current' } }],
['@babel/preset-react', { runtime: 'automatic' }],
],
};
```
Copy link
Owner

Choose a reason for hiding this comment

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

Since jest-preview is advocating for @swc-project (because of the speed gain). I think we should setup Jest with https://github.com/swc-project/jest. Refer to https://github.com/nvh95/jest-preview/blob/f0ef957bf89cbe9f0f94389ccf37d33e53b8a6f8/.swcrc and

'^.+\\.(ts|js|tsx|jsx)$': '@swc/jest',
for more

Comment on lines 48 to 67
Enable Jest Preview inside `jest.config.js`. The config options are largely up to you, but remember to remove/add the highlighted options as followed so Jest Preview could work properly.

```diff
module.exports = {
collectCoverageFrom: [
'**/*.{js,jsx,ts,tsx}',
'!**/*.d.ts',
'!**/node_modules/**',
],
moduleNameMapper: {
- // Handle CSS imports (with CSS modules)
- // https://jestjs.io/docs/webpack#mocking-css-modules
- '^.+\\.module\\.(css|sass|scss)$': 'identity-obj-proxy',
-
- // Handle CSS imports (without CSS modules)
- '^.+\\.(css|sass|scss)$': '<rootDir>/__mocks__/styleMock.js',
-
- // Handle image imports
- // https://jestjs.io/docs/webpack#handling-static-assets
- '^.+\\.(png|jpg|jpeg|gif|webp|avif|ico|bmp|svg)$/i': `<rootDir>/__mocks__/fileMock.js`,
Copy link
Owner

Choose a reason for hiding this comment

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

I think users usually want to copy-paste the very looong configuration, so if we change the markdown from diff to js, it would be very helpful. One more thing is code block on www.jest-preview.com and GitHub have the copy button, so it would help users a lot
copy button on code block

One suggestion is to put your message in the comments.

Comment on lines 102 to 104
// (Optional) Enable autoPreview so Jest Preview runs automatically
// whenever your test fails, without you having to do anything!
// autoPreview: true,
Copy link
Owner

Choose a reason for hiding this comment

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

We can enable it by default if there is no problem yet. It will be the default option soon anyway.

Comment on lines 84 to 100
},
transformIgnorePatterns: [
'/node_modules/',
- '^.+\\.module\\.(css|sass|scss)$',
],
};
```

Configure Jest Preview inside `jest.setup.js` (or any setup files specified in your `setupFilesAfterEnv` config), so Jest Preview knows which global CSS file to load. You can even set `autoPreview` to `true` so your failed test gets a preview automatically! 🤯

```js
import { jestPreviewConfigure } from 'jest-preview';

jestPreviewConfigure({
// An array of relative paths from the root of your project
externalCss: [
'app/styles/global.css',
Copy link
Owner

Choose a reason for hiding this comment

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

Too much manually configure. Can we create a CLI such as in #102. Can consider that option in the future PR

Comment on lines 8 to 10
// (Optional) Enable autoPreview so you get previewing for free,
// automatically without having to call debug(), whenever your test fails.
// autoPreview: true,
Copy link
Owner

Choose a reason for hiding this comment

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

In my opinion, this option should be enabled by default 😂

Comment on lines +6 to +9
// appDirectory: "app",
// assetsBuildDirectory: "public/build",
// serverBuildPath: "build/index.js",
// publicPath: "/build/",
Copy link
Owner

Choose a reason for hiding this comment

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

@thanhsonng Are those code from the template or you commented out them?

Copy link
Owner

Choose a reason for hiding this comment

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

@thanhsonng Do you remember this?

website/docs/examples/remix.md Show resolved Hide resolved
Copy link
Owner

@nvh95 nvh95 left a comment

Choose a reason for hiding this comment

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

LGTM!

@nvh95 nvh95 merged commit aa0c3b9 into main Jun 12, 2022
@nvh95 nvh95 deleted the remix-example branch June 12, 2022 17:29
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.

Add example for remix
2 participants