[Bug]: FeatureFlags queried on import wrong if package imported indirectly (Vite dev only) #13505

lee-chase · 2023-04-11T14:15:31Z

Package

@carbon/react

Browser

Chrome

Package version

1.26

React version

17

Description

When using Vite in dev mode when Carbon is referenced/loaded through a third party library Vite the feature flags queried on import are wrong.

This does not happen if using 'vite preview' which uses production built code.

NOTE: This will prevent @carbon/ibm-products from working with Vite and V11 support.

Reproduction/example

https://stackblitz.com/edit/github-l56ye7?file=src%2FApp.jsx,fake-package%2Fpackage.json,fake-package%2Findex.js,src%2FExample.jsx

Steps to reproduce

These steps are copied from carbon-design-system/ibm-products#2734

On loading this example may work (depending on state left in). If so follow the steps below to move 'fake-package' is into node_modules and run again.

Steps.

Open https://stackblitz.com/edit/github-l56ye7?file=src%2FApp.jsx,fake-package%2Fpackage.json,fake-package%2Findex.js,src%2FExample.jsx
Ctrl-C in the terminal window.
Copy fake package to node_modules using cp -R fake-package node_modules
Edit App.jsx to use fake-package from node_modules
Then npm run dev

NOTE: You may need to refresh the preview pane (not the whole page as this causes a reinstall of node_modules).

NOTE2: If you want to flip between the two scenarios you will need to delete the vite cache node_modules/.vite between runs.

Note if running this locally you will need to remove 'node_modules/.vite' between runs if changing node modules to bypass the cache.

Further testing...
Stack traces from the from local runs show that only Import time tests are impacted listing every component in @carbon/react that performs such a test and showing enable-v11-release evaluates to false. Only later when Dropdown performs a runtime test is the value correct.

[vite] connecting...
client.ts:53 [vite] connected.

index.js:353 flag check enable-css-grid false
index.js:354 console.trace
enabled2 @ index.js:354
enabled @ index.js:382
(anonymous) @ Column.js:58

index.js:353 flag check enable-v11-release false
index.js:354 console.trace
enabled2 @ index.js:354
enabled @ index.js:382
(anonymous) @ Slider.js:678
Tab.js:184 In tab {"FeatureFlags":{"flags":{}}}

index.js:353 flag check enable-v11-release false
index.js:354 console.trace
enabled2 @ index.js:354
enabled @ index.js:382
(anonymous) @ Tab.js:184

index.js:353 flag check enable-v11-release false
index.js:354 console.trace
enabled2 @ index.js:354
enabled @ index.js:382
(anonymous) @ Tab.js:187

index.js:353 flag check enable-v11-release false
index.js:354 console.trace
enabled2 @ index.js:354
enabled @ index.js:382
(anonymous) @ index.js:12

index.js:353 flag check enable-v11-release false
index.js:354 console.trace
enabled2 @ index.js:354
enabled @ index.js:382
(anonymous) @ index.js:11

index.js:353 flag check enable-v11-release false
index.js:354 console.trace
enabled2 @ index.js:354
enabled @ index.js:382
(anonymous) @ PasswordInput.js:252

index.js:353 flag check enable-v11-release false
index.js:354 console.trace
enabled2 @ index.js:354
enabled @ index.js:382
(anonymous) @ TextInput.js:257

index.js:353 flag check enable-v11-release false
index.js:354 console.trace
enabled2 @ index.js:354
enabled @ index.js:382
(anonymous) @ Accordion.js:72

index.js:353 flag check enable-v11-release false
index.js:354 console.trace
enabled2 @ index.js:354
enabled @ index.js:382
(anonymous) @ Button.Skeleton.js:60

index.js:353 flag check enable-v11-release false
index.js:354 console.trace
enabled2 @ index.js:354
enabled @ index.js:382
(anonymous) @ FileUploaderButton.js:190

index.js:353 flag check enable-v11-release false
index.js:354 console.trace
enabled2 @ index.js:354
enabled @ index.js:382
(anonymous) @ FileUploaderItem.js:116

index.js:353 flag check enable-v11-release false
index.js:354 console.trace
enabled2 @ index.js:354
enabled @ index.js:382
(anonymous) @ FileUploader.js:231

index.js:353 flag check enable-v11-release false
index.js:354 console.trace
enabled2 @ index.js:354
enabled @ index.js:382
(anonymous) @ FileUploader.js:275

index.js:353 flag check enable-v11-release false
index.js:354 console.trace
enabled2 @ index.js:354
enabled @ index.js:382
(anonymous) @ FileUploader.js:282

index.js:353 flag check enable-v11-release false
index.js:354 console.trace
enabled2 @ index.js:354
enabled @ index.js:382
(anonymous) @ SelectItemGroup.js:56
b4-index.js:1 b4 index.js
index.js:221 @carbon/react index.js
index.js:3 fake-package index.js
Example.jsx:16 example under test
Tabs.js:80 In tabs {"FeatureFlags":{"flags":{}}}
warning.js:24 Warning: This prop syntax has been deprecated. Please use the new `aria-label`.
warning2 @ warning.js:24
checker @ deprecate.js:20
checkPropTypes @ react.development.js:1916
validatePropTypes @ react.development.js:2136
createElementWithValidation @ react.development.js:2240
ThemeDropdown @ ThemeDropdown.jsx:16
renderWithHooks @ react-dom.development.js:14985
mountIndeterminateComponent @ react-dom.development.js:17811
beginWork @ react-dom.development.js:19049
beginWork$1 @ react-dom.development.js:23940
performUnitOfWork @ react-dom.development.js:22776
workLoopSync @ react-dom.development.js:22707
renderRootSync @ react-dom.development.js:22670
performSyncWorkOnRoot @ react-dom.development.js:22293
scheduleUpdateOnFiber @ react-dom.development.js:21881
updateContainer @ react-dom.development.js:25482
(anonymous) @ react-dom.development.js:26021
unbatchedUpdates @ react-dom.development.js:22431
legacyRenderSubtreeIntoContainer @ react-dom.development.js:26020
render @ react-dom.development.js:26103
(anonymous) @ main.jsx:26

index.js:353 flag check enable-v11-release true
index.js:354 console.trace
enabled2 @ index.js:354
useFeatureFlag @ index.js:122
(anonymous) @ Dropdown.js:105
renderWithHooks @ react-dom.development.js:14985
updateForwardRef @ react-dom.development.js:17044
beginWork @ react-dom.development.js:19098
beginWork$1 @ react-dom.development.js:23940
performUnitOfWork @ react-dom.development.js:22776
workLoopSync @ react-dom.development.js:22707
renderRootSync @ react-dom.development.js:22670
performSyncWorkOnRoot @ react-dom.development.js:22293
scheduleUpdateOnFiber @ react-dom.development.js:21881
updateContainer @ react-dom.development.js:25482
(anonymous) @ react-dom.development.js:26021
unbatchedUpdates @ react-dom.development.js:22431
legacyRenderSubtreeIntoContainer @ react-dom.development.js:26020
render @ react-dom.development.js:26103
(anonymous) @ main.jsx:26

Tabs.js:80 In tabs {"FeatureFlags":{"flags":{}}}

Suggested Severity

Severity 2 = User cannot complete task, and/or no workaround within the user experience of a given component.

Application/PAL

@carbon/ibm-products

Code of Conduct

I agree to follow this project's Code of Conduct
I checked the current issues for duplicate problems

The text was updated successfully, but these errors were encountered:

lee-chase · 2023-04-11T14:38:54Z

The most likely culprit is FeatureFlags.merge is. not called until later. Perhaps a sideEffects issue?

tay1orjones · 2023-04-12T19:19:08Z

@lee-chase I think you're onto something with suspecting sideEffects.

The feature flags code is currently listed in sideEffects, and so should always be included

carbon/packages/react/package.json

Lines 129 to 131 in a95ac1d

    
           "es/feature-flags.js", 
        
           "lib/feature-flags.js", 
        
           "src/feature-flags.js",

When @carbon/react is referenced through an intermediary package, like fake-package in your example, I believe Vite then treats @carbon/react as a dependency and pre-builds it using esbuild. It's possible the inverse is true though, I'm not exactly sure how Vite determines code to be dependency code or not. In either case, it appears Vite is improperly handling the import of feature flag code in dev vs prod since the two differ in what tools they use to prebundle dependencies between the two environments.

For the production builds, Vite uses Rollup, which interestingly enough had a recent update to properly respect sideEffects vitejs/vite#7635, rollup/rollup#4867

I see your example uses Vite v2 - I'm curious if you see the same behavior when using Vite v3? Something mentioned in the changelog caught my eye

Vite now allows the use of esbuild to optimize dependencies during build time avoiding the need of @rollup/plugin-commonjs, removing one of the difference id dependency handling between dev and prod.

tay1orjones · 2023-04-12T19:31:47Z

@lee-chase Also on the Vite docs about pre-bundling, I found this

Dependency pre-bundling only applies in development mode, and uses esbuild to convert dependencies to ESM. In production builds, @rollup/plugin-commonjs is used instead.

So you might also try customizing the pre-build behavior or explicitly turning off tree shaking when vite uses esbuild via the optimizeDeps.esbuildOptions config option.

lee-chase · 2023-04-17T16:08:20Z

@tay1orjones can we not simply update Carbon to replace the enable-v11-release as a feature flag with a enable-v10-release flag?

tay1orjones · 2023-04-19T20:20:25Z

@lee-chase Switching the flag to be an inverse "false by default" would just sidestep the problem, right? I think the same issue will occur in Vite for any feature flag that gets turned on by default in the future.

lee-chase · 2023-04-20T16:15:54Z

@lee-chase Switching the flag to be an inverse "false by default" would just sidestep the problem, right? I think the same issue will occur in Vite for any feature flag that gets turned on by default in the future.

Yes, just a sidestep but it would unblock @carbon/ibm-products temporarily.

lee-chase · 2023-05-02T10:56:31Z

@tay1orjones having copied the sample, possibly from Carbon, I had not realised the version of Vite was so out of date.

Updating to 4.3.4 appears to fix this.

https://stackblitz.com/edit/github-l56ye7-goyx6j?file=src%2Fmain.jsx,index.html,package.json,vite.config.js,src%2Findex.scss,src%2FApp.jsx

tay1orjones · 2023-05-02T13:04:38Z

@lee-chase Great news! I too didn't realize Vite was so out of date. We need to update our examples to use v4.x #13683

tay1orjones · 2023-05-02T19:43:48Z

I'm going to close this out but happy to reopen if there's still issues. We could always explore turning the v11 flag on at the root to avoid it ever being set false.

carbon/packages/feature-flags/feature-flags.yml

Lines 21 to 24 in 36685a0

    
             - name: enable-v11-release 
        
               description: > 
        
                 Enable the features and functionality for the v11 Release 
        
               enabled: false

Right now it's currently turned on within @carbon/react

carbon/packages/react/src/feature-flags.js

Line 13 in 36685a0

'enable-v11-release': true,

lee-chase added the type: bug 🐛 label Apr 11, 2023

github-project-automation bot added this to Design System Apr 11, 2023

sstrubberg moved this to ⏱ Backlog in Design System Apr 11, 2023

sstrubberg added role: dev 🤖 package: @carbon/react @carbon/react labels Apr 11, 2023

tay1orjones self-assigned this Apr 26, 2023

sstrubberg added this to the 2023 Q2 milestone Apr 26, 2023

sstrubberg mentioned this issue Apr 26, 2023

Q2 Support Tracker [Experiment] #13510

Closed

tay1orjones closed this as completed May 2, 2023

github-project-automation bot moved this from ⏱ Backlog to ✅ Done in Design System May 2, 2023

tay1orjones mentioned this issue May 4, 2023

[Bug ]: Tabs component is not rendering correctly in Remix.run (SSR) #13702

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bug]: FeatureFlags queried on import wrong if package imported indirectly (Vite dev only) #13505

[Bug]: FeatureFlags queried on import wrong if package imported indirectly (Vite dev only) #13505

lee-chase commented Apr 11, 2023

lee-chase commented Apr 11, 2023 •

edited

Loading

tay1orjones commented Apr 12, 2023

tay1orjones commented Apr 12, 2023

lee-chase commented Apr 17, 2023 •

edited

Loading

tay1orjones commented Apr 19, 2023

lee-chase commented Apr 20, 2023

lee-chase commented May 2, 2023

tay1orjones commented May 2, 2023

tay1orjones commented May 2, 2023

[Bug]: FeatureFlags queried on import wrong if package imported indirectly (Vite dev only) #13505

[Bug]: FeatureFlags queried on import wrong if package imported indirectly (Vite dev only) #13505

Comments

lee-chase commented Apr 11, 2023

Package

Browser

Package version

React version

Description

Reproduction/example

Steps to reproduce

Suggested Severity

Application/PAL

Code of Conduct

lee-chase commented Apr 11, 2023 • edited Loading

tay1orjones commented Apr 12, 2023

tay1orjones commented Apr 12, 2023

lee-chase commented Apr 17, 2023 • edited Loading

tay1orjones commented Apr 19, 2023

lee-chase commented Apr 20, 2023

lee-chase commented May 2, 2023

tay1orjones commented May 2, 2023

tay1orjones commented May 2, 2023

lee-chase commented Apr 11, 2023 •

edited

Loading

lee-chase commented Apr 17, 2023 •

edited

Loading