vuejs / pinia

🍍 Intuitive, type safe, light and flexible Store for Vue using the composition api with DevTools support
https://pinia.vuejs.org
MIT License
12.66k stars 995 forks source link

Bundling with webpack includes the devtools code #1143

Closed eeedvisss closed 8 months ago

eeedvisss commented 2 years ago

Hi,

First thank you for this amazing Store for Vue!

Reproduction

We just started to convert our vuex3 store to pinia and noticed that we get errors and application doesn't start when trying to launch the app on chrome in version 54 and under(in versions chrome v55+ everything works as we would expect):

image

async/await was introduced in chrome v55.

Steps to reproduce the behavior

Our environment:

This can be reproduced with a clean vue install.

Reproduction repository with a clean vue 2 app(pinia is initialised in src/App.vue): https://github.com/eeedvisss/pinia-test

Expected behavior

Application should work in chrome 54.

Actual behavior

Application breaks in chrome versions 54 and under.

Additional information

Some questions from our side:

Why are @vue/devtools-api methods being called in production directly with code that is not compatible with older browsers? Should pinia work on browsers earlier than chrome v55 or maybe just our pinia setup is wrong? Is it possible to turn off calls to @vue/devtools-api in production with something like "process.env.NODE_ENV === 'production'"?

Thanks!

posva commented 2 years ago

Yeah, the devtools code should be stripped off during minification. I wonder why it stays there

posva commented 2 years ago

Should be fixed in 0e9ffec I think that one should still configure webpack to resolve pinia to pinia/dist/pinia.mjs, the one in CJS doesn't seem to be properly tree shaken.

eeedvisss commented 2 years ago

@posva Sorry, but still getting the same error even with pinia 2.0.12, do we need extra configuration in webpack config? image

posva commented 2 years ago

This is quite weird. Something is odd with the webpack config generated by the CLI regarding minification. I verified the output code in pinia is correct so I won't be looking further into this because it does work with Vite but for anybody looking into this, it seems the mjs version is being used by webpack (which is correct), but some settings in the minification are not. Take a look at the optimization.minimizer option and start from there.

bponomarenko commented 1 year ago

@posva This issue just happened to me to me as well, though we could find the solution for it.

The logic to include devtools plugin or not is based on the USE_DEVTOOLS variable, which depends on this piece of the logic:

__DEV__ || __FEATURE_PROD_DEVTOOLS__

which would be transformed to the following code for the pinia.mjs dist:

__DEV__ || (typeof __VUE_PROD_DEVTOOLS__ !== 'undefined' && __VUE_PROD_DEVTOOLS__)

In our webpack configuration we were setting __DEV__: false with ProvidePlugin, but not __VUE_PROD_DEVTOOLS__. As a result

__DEV__ || (typeof __VUE_PROD_DEVTOOLS__ !== 'undefined' && __VUE_PROD_DEVTOOLS__)

would be transformed to

false || (typeof __VUE_PROD_DEVTOOLS__ !== 'undefined' && __VUE_PROD_DEVTOOLS__)

and minimizer wouldn't be able to tree-shake this part of the code since __VUE_PROD_DEVTOOLS__ would be considered a "global variable".

Setting __VUE_PROD_DEVTOOLS__: false with the ProvidePlugin will eventually allow to transform

__DEV__ || (typeof __VUE_PROD_DEVTOOLS__ !== 'undefined' && __VUE_PROD_DEVTOOLS__)

to

false || (typeof false !== 'undefined' && false)

which minimizer would treat as "falsy expression" and would remove it from the production dist.

I was trying to find any documentation about __VUE_PROD_DEVTOOLS__ on the website but doesn't seem it exists. It would be great to explain it since devtools code is by default added to the production output when Pinia is built with Webpack/Rollup/etc. with custom configuration.

posva commented 1 year ago

I think we cannot use a variable in the pinia source code and expect it to enable code removal, it has to be inlined everywhere it's needed. So I think we need to move the USE_DEVTOOLS to the list of replacements. It will be a long one but hopefully, it should work with more minifiers. Another solution is revisiting where this expression is used and split into multiple nested ifs that can be stripped more easily.

bponomarenko commented 1 year ago

Thanks for re-opening the issue. I guess "inlining USE_DEVTOOLS" may improve the situation.

Do you think there is also a need to add documentation for use cases when Pinia is built in the projects with custom build tool configuration?

posva commented 1 year ago

Do you think there is also a need to add documentation for use cases when Pinia is built in the projects with custom build tool configuration?

What do you mean?

bponomarenko commented 1 year ago

My understanding is that "inlining USE_DEVTOOLS" would result in code like this

if (USE_DEVTOOLS) {
  // dev tools specific logic
}

to be actually transformed to something like the following code for *.mjs dist file:

if (((process.env.NODE_ENV !== 'production') || (typeof __VUE_PROD_DEVTOOLS__ !== 'undefined' && __VUE_PROD_DEVTOOLS__)) && !(process.env.NODE_ENV === 'test') && IS_CLIENT) {
  // dev tools specific logic
}

Which means that __VUE_PROD_DEVTOOLS__ and IS_CLIENT should be added to the list of replacements so that when Pinia is compiled in the target project, they would be properly replaced and correctly handled by the minifier.

I guess that is already the case for the official Pinia plugins for Vite, but they would have to be explicitly set for the projects with custom build tool configurations (Webpack, Rollup, etc.). So it might be useful to have a mention of those values since would affect output bundle content. Did I get it right?

posva commented 1 year ago

the devtools one should also be set in webpack. These are documented in Vue, they shouldn't be in pinia docs

vergilfromadyen commented 1 year ago

These are documented in Vue, they shouldn't be in pinia docs

I believe this is the case with Vue 3.0.0-rc.3 and onwards and not for Vue 2.

bponomarenko commented 1 year ago

These are documented in Vue, they shouldn't be in pinia docs.

That's a good point, found some info about this flag in the plugins guide and FAQ section for the Vue devtools itself. It is a pity though that there is no information about __VUE_PROD_DEVTOOLS__ on their website. Also Vue3 is explicitly mentioned and not Vue2:

By default, Vue 3 doesn't include the devtools-related code in production. It uses the __VUE_PROD_DEVTOOLS__ environment variable...

But that is not Pinia's issue, of course. Thanks for the clarification @posva ;)

posva commented 1 year ago

Agree, even though it's an advanced feature that is expected to just work, it should be visible in docs

brandonburkett commented 1 year ago

Unsure if this is related, but is this also why pineapples and other emoji are in the comments within dist/pinia.mjs when they are not in dist/pinia.prod.cjs? I see them in our minified final app.js output.

I also noticed object spread operator is used in dist/pinia.mjs (which breaks Edge 15 -18), but it's not used in dist/pinia.prod.cjs.

Is there supposed to be a prod version of pinia.mjs or is the intention for us to transpile it ourselves? The cjs version has an import toggle based on NODE_ENV, but I don't see this equivalent for the mjs version.

I am using webpack 5 + vue 2.7.

EDIT: Currently we have a webpack alias set to the cjs export that includes the NODE_ENV toggle. which solves all of the above.

BboyAkers commented 1 year ago

I'll take a look at this!

gregpalaci commented 2 months ago

Hmm seems like you can't turn off devtools with pinia + nuxt2 bridge I'm trying NODE_ENV=production; yarn build --devtools && yarn start and in nuxt.config vue: { config: { productionTip: true, devtools: true, }, }, I've tried false also I don't want devtools on but I really don't want the error. I also installed the devtools in package.json "@vue/devtools": "^7.1.3", I guess the only other thing is adding it as a global but I rather disable it?