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

Bug: Cannot resolve symbol s_c7PiYJ1s0GU... Error code (31) - Namespace conflicts #519

Open
maiieul opened this issue Nov 18, 2023 · 6 comments

Comments

@maiieul
Copy link
Contributor

maiieul commented Nov 18, 2023

Error message:

Cannot resolve symbol s_c7PiYJ1s0GU in {
'02wMImzEAbk': [ 's_02wMImzEAbk', 'entry_QwikCityProvider-a7f704b4.js' ],
ClIa90D1LOw: [ 's_ClIa90D1LOw', 'entry_AccordionTrigger-0b3712d8.js' ],
moIlZJ0XpGY: [ 's_moIlZJ0XpGY', 'entry_AccordionTrigger-0b3712d8.js' ],
yqEZj1cracY: [ 's_yqEZj1cracY', 'entry_AccordionContent-4d7f7f97.js' ],
FVuPkVHsyn8: [ 's_FVuPkVHsyn8', 'entry_Slider-714836a4.js' ],
M0tH5WLqYIQ: [ 's_M0tH5WLqYIQ', 'entry_AccordionContent-4d7f7f97.js' ],
ZlkQX0dQ4iE: [ 's_ZlkQX0dQ4iE', 'entry_AccordionTrigger-0b3712d8.js' ],
py4L6HieTpw: [ 's_py4L6HieTpw', 'entry_AccordionTrigger-0b3712d8.js' ],
'0vphQYqOdZI': [ 's_0vphQYqOdZI', 'entry_RouterHead-adf3888c.js' ],
'8gdLBszqbaM': [ 's_8gdLBszqbaM', 'entry_Link-308277b9.js' ],
FpJ0F5de3jc: [ 's_FpJ0F5de3jc', 'entry_AccordionContent-4d7f7f97.js' ],
HLYQug4XlU4: [ 's_HLYQug4XlU4', 'entry_AccordionTrigger-0b3712d8.js' ],
Nk9PlpjQm9Y: [ 's_Nk9PlpjQm9Y', 'entry_GetForm-bc74dc7f.js' ],
TxCFOy819ag: [ 's_TxCFOy819ag', 'entry_QwikCityProvider-a7f704b4.js' ],
VKFlAWJuVm8: [ 's_VKFlAWJuVm8', 'entry_layout-f303f0c7.js' ],
WmYC5H00wtI: [ 's_WmYC5H00wtI', 'entry_QwikCityMockProvider-1dd1a1fb.js' ],
YgHXnQO1y0I: [ 's_YgHXnQO1y0I', 'entry_AccordionItem-0440f64c.js' ],
e0ssiDXoeAM: [ 's_e0ssiDXoeAM', 'entry_RouterOutlet-4c6a613c.js' ],
p190h1R2pFo: [ 's_p190h1R2pFo', 'entry_AccordionRoot-35cc58b1.js' ],
sgaPoRbdtIY: [ 's_sgaPoRbdtIY', 'entry_Slider-714836a4.js' ],
tntnak2DhJ8: [ 's_tntnak2DhJ8', 'entry_root-1b9ed647.js' ],
wrpW1Kq03pA: [ 's_wrpW1Kq03pA', 'entry_Slider-714836a4.js' ],
RPDJAz33WLA: [ 's_RPDJAz33WLA', 'entry_QwikCityProvider-a7f704b4.js' ],
jkiDTw0Rxa4: [ 's_jkiDTw0Rxa4', 'entry_AccordionContent-4d7f7f97.js' ],
A5bZC7WO00A: [ 's_A5bZC7WO00A', 'entry_routeActionQrl-a17b7b57.js' ],
DyVc0YBIqQU: [ 's_DyVc0YBIqQU', 'entry_spaInit-663033b0.js' ],
wOIPfiQ04l4: [ 's_wOIPfiQ04l4', 'entry_serverQrl-df2d3754.js' ],
BUbtvTyvVRE: [ 's_BUbtvTyvVRE', 'entry_QwikCityMockProvider-1dd1a1fb.js' ],
OOpUKBP0tL4: [ 's_OOpUKBP0tL4', 'entry_Slider-714836a4.js' ],
OPIX0WmIn3U: [ 's_OPIX0WmIn3U', 'entry_AccordionTrigger-0b3712d8.js' ],
WN9guyVeHSM: [ 's_WN9guyVeHSM', 'entry_Slider-714836a4.js' ],
aUMl0Mknykg: [ 's_aUMl0Mknykg', 'entry_AccordionTrigger-0b3712d8.js' ],
dGXzdRcdkdY: [ 's_dGXzdRcdkdY', 'entry_AccordionContent-4d7f7f97.js' ],
eBQ0vFsFKsk: [ 's_eBQ0vFsFKsk', 's_ebq0vfsfksk-371ab53b.js' ],
fX0bDjeJa0E: [ 's_fX0bDjeJa0E', 'entry_QwikCityProvider-a7f704b4.js' ],
i1Cv0pYJNR0: [ 's_i1Cv0pYJNR0', 'entry_Link-308277b9.js' ],
nED0ZS0dwY4: [ 's_nED0ZS0dwY4', 'entry_Slider-714836a4.js' ],
p9MSze0ojs4: [ 's_p9MSze0ojs4', 'entry_GetForm-bc74dc7f.js' ],
sqOFV6JnIxY: [ 's_sqOFV6JnIxY', 'entry_Slider-714836a4.js' ],
vCRJEICs24o: [ 's_vCRJEICs24o', 'entry_AccordionTrigger-0b3712d8.js' ],
zRbRiLzl1zc: [ 's_zRbRiLzl1zc', 'entry_AccordionTrigger-0b3712d8.js' ]
}
QWIK ERROR Error: Code(31)

@shairez @thejackshelton @wmertens So I've been able to narrow down the "cannot resolve symbol s_c7PiYJ1s0GU..." issue I got on qwikcn by consuming qwik-ui (the one I was showcasing on wednesday).

The problem is that build.rollupOptions.output.preserveModules creates namespace issues.

So with

    rollupOptions: {
      output: {
        preserveModules: true,
        preserveModulesRoot: 'packages/kit-headless/src',
      },
    },

If I now create a component and call it with the same name as one of the headless-kit's components:

export const AccordionRoot = component$(() => {
  return <div></div>;
});

This newly defined AccordionRoot will override the one exported by the headless-kit.

I found this bug happening when there is a useVisibleTask$ inside of the headless-kit's component, I guess because useVisibleTask$ will create a separate symbol that tries to access the headless-kit's component, but it isn't there anymore as it got overwritten by my component. Maybe the bug can happen in other circumstances as well.

My understanding is that we need this preserveModules config in the headless library to avoid the big bundle issue. Looks like this is because without preserveModules Qwik doesn't know how to bundle the already bundled headless components.

My workaround for now would be to rename the headless components to something like HeadlessAccordionRoot and so on..

Would be better DX though if we can keep the normal naming, no? I'll try to see if I can come up with a better workaround tomorrow 🙃

repro: https://github.com/maiieul/qwik-ui-tabs-repro

You need to run the production build to see the error (pnpm preview)

Simply change the name of the AccordionRoot component to MyAccordion or any other name not already being used by the headless-kit and the error is gone.

@maiieul
Copy link
Contributor Author

maiieul commented Nov 19, 2023

Misko says this is an optimizer issue: QwikDev/qwik#5097

This still happens in Qwik version 1.2.18 btw.

@maiieul
Copy link
Contributor Author

maiieul commented Nov 19, 2023

Spent the entire day trying to find a solution but I believe this is only solvable by changing the optimizer.

For the moment we have a few options:

  1. Keep the names as is

    • 👎 This will force consumers of the headless library to come-up with their own non-generic names when creating their own reusable components on top of the headless components.
    • 👎 Some consumers will unavoidably get the error when using a name already being used by the headless-kit, even if this is well documented.
  2. Change the headless components names with a prefix/suffix to something like AccordionRootHeadless or QUIAccordionRoot

    • 👍 This will make the headless components more distinguishable,
    • 👎 still a workaround that will require quite some work on the consumer to rename all the components (now and also later if we ever get back to the current naming when the issue gets fixed).
  3. Separate each component into its own package like radix/ui.

    • 👎 This will prevent consumers from importing everything from the same package
    • 👍 👍 This should avoid naming conflicts and having to use preserveModules
    • 👎 this is a quite controversial approach that can add up in maintenance cost for mainteners and consumers https://twitter.com/thesegunadebayo/status/1719050841234030799
  4. Find a way to separate each headless component into a subpackage with something like import AccordionRoot from '@qwik-ui/headless/accordion';

    • 👎 This will prevent consumers from importing everything from the same package
    • 👍 👍 This should avoid naming conflicts and having to use preserveModules.
    • 👍 👎 If the optimizer issue gets fixed, we can then export everything from the same package and consumers will just have to update their import paths. That should be easier than to update all the component names (1 or 2).
    • 👍 This should be easier to maintain than 3, and be more future-proof than 1 or 2.

    I'm not sure 4 is possible, but if it is, it is my preferred workaround as it would currently provide the best DX and perf for the consumers of the library. I think this can be done by providing an array of entry files. I will investigate this further.

@thejackshelton
Copy link
Contributor

thejackshelton commented Nov 20, 2023

Awesome find @maiieul! Out of all of these 4 definitely seems like the ideal workaround. The user would have to add a submodule for each component, which can be tedious, but I think that is a better alternative to the confusing namespace errors.

With that said though, if the optimizer issue is fixed, do we need preserveModules? Like you mentioned, that seems to be the option giving this tradeoff in the first place.

@shairez would love to hear your thoughts, especially with the PR #511. My understanding, is that we were trying to fix this in all libs, or change the library starter so that it properly tree shakes.

@maiieul
Copy link
Contributor Author

maiieul commented Nov 20, 2023

@thejackshelton Yes, after a good night's sleep, I realized that we also have another option, which is to not use preserveModules until the issue gets fixed in the optimizer. I would suggest doing that for now, and if the issue hasn't been fixed by the time we reach v1, then we can think this through again and see if it's worth using any of the workaround above with preserveModules.

Perhaps in the meantime we can improve tree-shaking on the big bundle. Maybe marking some functions as QRLs with the $ sign, and make sure that huge JSON files don't get bundled in it. Maybe that's why you put '@floating-ui/dom', 'country-list-json', 'libphonenumber-js' in RollupOptions.external @shairez ? Also maybe #511 improves tree-shaking (without preserverModules) a little bit @shairez?

@shairez
Copy link
Contributor

shairez commented Nov 20, 2023

@maiieul my work on fixing the bundle size issue was delayed because after upgrading to the latest version we discovered that the Cypress tests do not run, now that @dmitry-stepanenko is going to look at the cypress issue, I have more time to integrate it into main so we could release a version.

Have you checked my work on #511 ?

I think we should pair up and see if it's still doesn't solve the issue, because what I worked on might solve it as currently in main it doesn't reflect all of the solutions that my PR added

@maiieul
Copy link
Contributor Author

maiieul commented Nov 29, 2023

Core issue: QwikDev/qwik#5473

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

No branches or pull requests

3 participants