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

Remove workaround fix for storybook #10696

Closed
samwhale opened this issue Feb 23, 2022 · 4 comments
Closed

Remove workaround fix for storybook #10696

samwhale opened this issue Feb 23, 2022 · 4 comments
Labels
P3 Nice to have Type: Infrastructure Changes impacting testing infrastructure or build tooling

Comments

@samwhale
Copy link
Contributor

samwhale commented Feb 23, 2022

Task Description

This is a follow-up to #6304

In #5792 storybook was upgraded to version 6.4. However, webpack was choking when running storybook because the project is ESM but there is a plugin that, when built, uses require because it is compiled to CJS.

The issue is further explored in this thread:

  1. If you specify your project as { "type": "module" }, webpack doesn't want to see require() any more
  2. The entry point(s) we create on your behalf as SB (generated from virtualModuleStory.template.js, which was compiled to CJS) makes use of require()
  3. In 6.3, as that file was "put" in ./.storybook, if you put a package.json in there declaring a non "module" package, WP is happy again.

To fix this, workaround code was added into main.cjs which replaces the file extensions on the plugins that are getting compiled to CJS.

Once storybook 6.5 is released and the workaround code is not needed anymore, remove the code.

@swissspidy swissspidy added Pod: WP & Infra Type: Infrastructure Changes impacting testing infrastructure or build tooling Status: Blocked On hold for the time being labels Feb 23, 2022
@swissspidy
Copy link
Collaborator

Apparently the release is slated for end of April: storybookjs/storybook#16797

@samwhale
Copy link
Contributor Author

samwhale commented Mar 30, 2022

Yay!!

I've been working on #10788 this morning as well. Looks like HMR isn't broken 🥳 but the log warnings seem to be related to the same code that was added in ./.storybook/main.cjs to prevent storybook from crashing

Trying to verify that upgrading locally to the alpha version fixes both of these tickets. Then when the new version is released, we can be certain that both issues will be fixed.

Edit: it is broken

@samwhale
Copy link
Contributor Author

Confirmed that the alpha release will fix this issue. However, it seems that the logs in #10788 aren't fixed by the upgrade. Will keep digging.

@samwhale
Copy link
Contributor Author

samwhale commented Mar 30, 2022

Note: While upgrading I ran into these errors:

ModuleDependencyError: The requested module './story' contains conflicting star 
exports for the name '__namedExportsOrder' with the previous requested module './fonts'

ModuleDependencyError: The requested module './utils' contains conflicting star 
exports for the name '__namedExportsOrder' with the previous requested module './constants'

ModuleDependencyError: The requested module './settings' contains conflicting star
exports for the name '__namedExportsOrder' with the previous requested module './textConstants'

ModuleDependencyError: The requested module './wpAdmin' contains conflicting star
exports for the name '__namedExportsOrder' with the previous requested module './textConstants'

For some reason, the compilation is failing due to some * imports. Replacing the imports in the following files worked for me:

// .storybook/stories/playground/story-editor/api/index.js

export { getFonts } from './fonts'; // export * from './fonts';
export { getMedia } from './media'; // export * from './media';
export { saveStoryById } from './story'; // export * from './story';
// packages/wp-dashboard/src/constants/index.js

export { SUCCESS, ERRORS } from './textConstants'; // export * from './textConstants';
export {
  EDITOR_SETTINGS_ROUTE,
  LEFT_RAIL_SECONDARY_NAVIGATION,
  AD_NETWORK_TYPE,
  ARCHIVE_TYPE
} from './settings'; // export * from './settings';
export {
  TOOLBAR_HEIGHT,
  MENU_WIDTH,
  MENU_FOLDED_WIDTH
} from './wpAdmin'; // export * from './wpAdmin';
// packages/fonts/src/index.js

export { CURATED_FONT_NAMES } from './constants'; // export * from './constants';
export { getFontCSS, getGoogleFontURL } from './utils'; // export * from './utils';

Also, assets weren't getting loaded in properly
image

Adding some info here in case these errors are seen when upgrading.

@swissspidy swissspidy added the P3 Nice to have label May 16, 2022
@swissspidy swissspidy changed the title Remove workaround fix for storybook when storybook 6.5 is released Remove workaround fix for storybook May 19, 2022
@swissspidy swissspidy removed the Status: Blocked On hold for the time being label May 19, 2022
@swissspidy swissspidy added P2 Should do soon P3 Nice to have and removed P3 Nice to have P2 Should do soon labels Sep 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 Nice to have Type: Infrastructure Changes impacting testing infrastructure or build tooling
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants