storybookjs / react-native

📓 Storybook for React Native!
https://storybook.js.org
MIT License
1.07k stars 155 forks source link

Fix webpack5 and @storybook/addon-storyshots compatibility error, and a typo #442

Closed leonardo-fc closed 1 year ago

leonardo-fc commented 1 year ago

How to test the bug and the fix

git clone https://github.com/leonardo-fc/patched-react-native-storybook-test.git /tmp/patched-react-native-storybook-test
cd /tmp/patched-react-native-storybook-test
yarn install

# Runs the current @storybook/react-native-server with webpack v4 and v5
# and the patched one with webpack v4 and v5
yarn test:server

# Runs @storybook/addon-storyshots with the current and the patched @storybook/react-native
yarn test:storyshots

# Take a look at the tests files to see how things are being tested
dannyhw commented 1 year ago

Hey thanks so much for this contribution 🙏.

Just at first glance I'm wondering does the server work with this change for webpack 5? I had some problems when trying this last time.

leonardo-fc commented 1 year ago

Updated patched-react-native-storybook-test to include start storybook scripts:

yarn patched-storybook-webpack5:ios

https://user-images.githubusercontent.com/124659450/225424134-5e959eab-cffc-4a7e-9806-d1183f616f4b.mp4

dannyhw commented 1 year ago

@leonardo-fc this is awesome! Thanks for the video :).

Though I think maybe we should actually use webpack5 by default and only use webpack4 if its specified

leonardo-fc commented 1 year ago

Agree

leonardo-fc commented 1 year ago

@storybook/core-server uses webpack5 or webpack4 based on the config file So, should we do something like this? to make webpack5 the default

const mainFilePath = path.join(options.configDir, 'main');

fs.writeFileSync('/somewhere/main.js', `
const config = require(${mainFilePath});

config.core ??= { builder: 'webpack5' };

module.exports = config;
`);

options.configDir = '/somewhere';
dannyhw commented 1 year ago

@leonardo-fc unfortunately I can't get this to work in the example project in this repository so I can't validate these changes.

Would you be able to get it running with the example project?

dannyhw commented 1 year ago

funny enough I was actually able to get it to build moments after sending that...

dannyhw commented 1 year ago

@leonardo-fc in future please do your testing in the example project here so that its easier for me to test.

I was however able to get this working and its looking good so thanks for figuring this one out.

socket-security[bot] commented 1 year ago

Socket Security Pull Request Report

Dependency issues detected: If you merge this pull request, you will not be alerted to the instances of these issues again.

⚠️ New author

A new npm collaborator published a version of the package for the first time. New collaborators are usually benign additions to a project, but do indicate a change to the security surface area of a package.

Scrutinize new collaborator additions to packages because they now have the ability to publish code into your dependency tree. Packages should avoid frequent or unnecessary additions or changes to publishing rights.

Package New Author Previous Author Source
html-minifier-terser@6.1.0 (added) sibiraj-s danielruf package.json via @storybook/builder-webpack5@6.5.16, @storybook/manager-webpack5@6.5.16, examples/expo-example/package.json via @storybook/builder-webpack5@6.5.16, @storybook/manager-webpack5@6.5.16
Pull request report summary
Issue Status
Install scripts ✅ 0 issues
Native code ✅ 0 issues
Bin script confusion ✅ 0 issues
Bin script shell injection ✅ 0 issues
Shell access ✅ 0 issues
Uses eval ✅ 0 issues
Unresolved require ✅ 0 issues
Invalid package.json ✅ 0 issues
HTTP dependency ✅ 0 issues
Git dependency ✅ 0 issues
GitHub dependency ✅ 0 issues
New author ⚠️ 1 issue
Potential typo squat ✅ 0 issues
Known Malware ✅ 0 issues
Telemetry ✅ 0 issues
Protestware/Troll package ✅ 0 issues
AI detected malware ✅ 0 issues
Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

  • @SocketSecurity ignore html-minifier-terser@6.1.0

Powered by socket.dev