-
Notifications
You must be signed in to change notification settings - Fork 8
fix: Compatibility issue between Common/ES Modules #83
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, but some things seem to be broken that are working in the main branch
@@ -54,7 +51,7 @@ export const viteFinal: StorybookConfig["viteFinal"] = async ( | |||
|
|||
export const previewAnnotations: StorybookConfig["previewAnnotations"] = ( | |||
entry = [], | |||
) => [...entry, require.resolve("storybook-framework-qwik/preview.js")]; | |||
) => [...entry, join(dirname(fileURLToPath(import.meta.url)), "../preview.js")]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change seems to make yarn workspace qwik-app storybook
fail to load for me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5:00:14 PM [vite] Pre-transform error: Failed to resolve import "/Users/benjaminkindle/Developer/Web/storybook-framework-qwik/packages/storybook-framework-qwik/preview.js" from "/virtual:/@storybook/builder-vite/vite-app.js". Does the file exist?
5:00:14 PM [vite] Internal server error: Failed to resolve import "/Users/benjaminkindle/Developer/Web/storybook-framework-qwik/packages/storybook-framework-qwik/preview.js" from "/virtual:/@storybook/builder-vite/vite-app.js". Does the file exist?
Plugin: vite:import-analysis
File: /virtual:/@storybook/builder-vite/vite-app.js:7:81
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice i will take a look ASAP
@@ -15,7 +15,7 @@ | |||
"watch": "yarn workspace storybook-framework-qwik watch", | |||
"build": "yarn workspace storybook-framework-qwik build", | |||
"lint-all": "yarn workspaces foreach run fmt.check && yarn workspaces foreach run lint", | |||
"storybook": "yarn workspace qwik-app storybook", | |||
"storybook": "yarn workspace qwik-lib storybook", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please revert this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing mb
Is this still in progress? |
This PR fix #82.
Changes made:
preset.js
to Windows users (like me)previewAnnotations
function now do not relay on require anymorewrapForPnP
function was removed since StoryBook now handle imports by its own