Skip to content

Build initially fails when using both babel and eslint but pass on second attempt #5399

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

Closed
gdmoore opened this issue Apr 17, 2020 · 24 comments
Closed

Comments

@gdmoore
Copy link

gdmoore commented Apr 17, 2020

Version

4.3.1

Reproduction link

https://github.com/gdmoore/vue-cli-babel-err

Environment info

  System:
    OS: Windows 10 10.0.18363
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
  Binaries:
    Node: 12.15.0 - C:\Program Files\nodejs\node.EXE
    Yarn: Not Found
    npm: 6.14.4 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: 44.18362.449.0
  npmPackages:
    @vue/babel-helper-vue-jsx-merge-props:  1.0.0
    @vue/babel-plugin-transform-vue-jsx:  1.1.2
    @vue/babel-preset-app:  4.3.1
    @vue/babel-preset-jsx:  1.1.2
    @vue/babel-sugar-functional-vue:  1.1.2
    @vue/babel-sugar-inject-h:  1.1.2
    @vue/babel-sugar-v-model:  1.1.2
    @vue/babel-sugar-v-on:  1.1.2
    @vue/cli-overlay:  4.3.1
    @vue/cli-plugin-babel: ~4.3.1 => 4.3.1
    @vue/cli-plugin-eslint: ~4.3.1 => 4.3.1
    @vue/cli-plugin-router: ~4.3.1 => 4.3.1
    @vue/cli-plugin-vuex: ~4.3.1 => 4.3.1
    @vue/cli-service: ~4.3.1 => 4.3.1
    @vue/cli-shared-utils:  4.3.1
    @vue/component-compiler-utils:  3.1.2
    @vue/eslint-config-prettier: ^6.0.0 => 6.0.0
    @vue/preload-webpack-plugin:  1.1.1
    @vue/test-utils: 1.0.0-beta.31 => 1.0.0-beta.31
    @vue/web-component-wrapper:  1.2.0
    eslint-plugin-vue: ^6.2.2 => 6.2.2
    vue: ^2.6.11 => 2.6.11
    vue-eslint-parser:  7.0.0
    vue-hot-reload-api:  2.3.4
    vue-loader:  15.9.1
    vue-router: ^3.1.6 => 3.1.6
    vue-style-loader:  4.1.2
    vue-template-compiler: ^2.6.11 => 2.6.11
    vue-template-es2015-compiler:  1.9.1
    vuex: ^3.1.3 => 3.1.3
  npmGlobalPackages:
    @vue/cli: Not Found


Steps to reproduce

npm install
npm run build
<error "Unexpected console statement  no-console">
npm run build
<success>

What is expected?

Eslint configured for no-console but babel configured with transform-remove-console, so the console statement should not cause a build error.

Build results should be consistent on every build.

What is actually happening?

Build fails on first attempt, succeeds on the second, with no code changes.

Any changes to main.js (even trivial ones such as adding or removing comments) will cause the build to break and require two builds again for success.


This is a minified case from my actual project, which was generated using vue create and choosing Babel and ESLint.

I suspect this is a bug with caching or in determining the order to run eslint and babel.

@javimendezona
Copy link

I'm having the same issue. Sharing a thread from the vue forum where others are reporting the same: https://forum.vuejs.org/t/remove-console-not-always-applied/87810/2

@OddDev
Copy link

OddDev commented May 7, 2020

We also just reproduced that.
It's reproducible with several different combinations of selected features (yarn/npm, ts/js etc.pp.).

(MacOS 10.14.6 (18G3020))

@activenode
Copy link

This actually breaks our current build setup.

@OddDev
Copy link

OddDev commented May 7, 2020

It also happens when you do not choose Babel.

@OddDev
Copy link

OddDev commented May 7, 2020

Steps to reproduce:

  1. Clone https://github.com/OddDev/vue-fails-on-second-attempt
  2. cd into root
  3. Execute yarn build (or npm run build or whatever)
  4. Mention that it fails (correctly)
  5. Execute yarn build (or npm run build or whatever) again
  6. Mention that it runs through (but should fail again obviously)

@activenode
Copy link

Another gained insight:

  1. Run yarn build with eslint failures
  2. build fails ✅
  3. Run yarn build again
  4. build succeeds ❌
  5. Delete node_modules/.cache
  6. Run yarn build
  7. build fails ✅

So it seems to be a caching issue

@activenode
Copy link

Apparently a workaround until Vue team fixes it is making the cache directory unwriteable:

rm -rf node_modules/.cache
mkdir node_modules/.cache
chmod -w node_modules/.cache

@javimendezona
Copy link

While there might be a workaround for local building, I haven't figured out a way to make it work with CI/CD workflows. This issue is blocking my ability to use Netlify's automated build and deploy feature from git merges. Just wanted to share that as I think it makes the case for prioritizing this issue (unless someone has a workaround)

@activenode
Copy link

activenode commented May 7, 2020

Couldn't you integrate this workaround in your CI/CD environment?
How is your CI setup given? Can you give details? So I might be able to help you here

@activenode
Copy link

I was digging a bit deeper and it most probably comes from either vue-loader or ts-loader or both since when I delete either of those the errors come back as accepted (i need to remove both from .cache to have a proper result).

I also tested around with @vue/cli-plugin-eslint/index.js but the eslint-plugin does not seem to be the evil box here.

@activenode
Copy link

Now more info on that:

Both vue-loader and ts-loader cache trigger the problem but in different ways. When you are using ts-loader (so you have TypeScript in your project) then once the ts-loader .cache entry is there it will "forget" to lint the ts files.

Analogously this applies for vue-loader and its .vue files and its scripts within those vue files.

Summed up:
If you dont use typescript you need to delete the .cache/vue-loader but if you use TypeScript you need to delete both .cache/vue-loader and .cache/ts-loader

@gdmoore
Copy link
Author

gdmoore commented May 8, 2020

Now more info on that:

Both vue-loader and ts-loader cache trigger the problem but in different ways. When you are using ts-loader (so you have TypeScript in your project) then once the ts-loader .cache entry is there it will "forget" to lint the ts files.

Analogously this applies for vue-loader and its .vue files and its scripts within those vue files.

Summed up:
If you dont use typescript you need to delete the .cache/vue-loader but if you use TypeScript you need to delete both .cache/vue-loader and .cache/ts-loader

This is not the same issue as I describe in the issue summary. I am not using typescript, and I do not have a vue-loader folder in .cache.

It's possible that the issues are related but this is not a workaround for the issue I reported.

@gdmoore
Copy link
Author

gdmoore commented May 8, 2020

My issue is actually the opposite with respect to the .cache folder.

With the vue-cli-babel-err repo in the issue summary, if I delete the cache before building, my build will always fail with Unexpected console statement. The cache must exist, or else I cannot successfully build.

It seems likely that depending on your loader usage and linting levels, the build results may actually be different on the first build versus every subsequent build.

@activenode
Copy link

activenode commented May 8, 2020

@gdmoore As you can see above a bare repo was created with the minimal set given through vue create ... so we figured it actually appears with nearly ANY setup you do with vue currently as long as you have linting.

The expected result is: [The build MUST fail because console.log are set to error mode]. That is what is important to understand @gdmoore.
But the 2nd time it runs it does NOT FAIL even though it SHOULD!
So Vue is giving a false positive which is dangerous.

Here is a new and cleaner-to-apply workaround:

// vue.config.js

module.exports = {
  chainWebpack: config => {
    config.module.rule('vue').uses.delete('cache-loader');
    config.module.rule('js').uses.delete('cache-loader');
    config.module.rule('ts').uses.delete('cache-loader');
    config.module.rule('tsx').uses.delete('cache-loader');
  }
};

@gdmoore
Copy link
Author

gdmoore commented May 8, 2020

@activenode see the repository in the issue summary. main.js is a single line, which is console.log(...).

I believe you did not see that I have Babel configured with transform-remove-console so that line should always be removed. The expected result is that the build should succeed every time.

Also if you do build my repository, notice that when it builds successfully, the console.log statement is not present in the built app.[hash].hs. This means on a rebuild, Babel runs correctly and the build output is correct.

I suspect that the issue I reported and the issue you are experiencing are strongly related, but not identical.

@activenode
Copy link

activenode commented May 8, 2020

@gdmoore

So let me try to explain what is happening in your case and clarify my assumption why your issue is the same issue as our issue:

You assume that your plugins.push("transform-remove-console"); is properly working when you run it 2 times because after 2 times the build does not fail anymore.

This however is a "false positive". Your babel comes into effect to remove the console.log from the built file. The babel plugin however does NOT prevent the console.log to go through linting. Those are 2 different things and the linting happens before babelfying. This also explains why you get an error because your .eslintrc looks like this:

module.exports = {
  ...
  rules: {
    "no-console": process.env.NODE_ENV === "production" ? "error" : "off",
    "no-debugger": process.env.NODE_ENV === "production" ? "error" : "off",
    "no-alert": "error"
  }
};

but it should look like this in your example:

module.exports = {
  ...
  rules: {
    "no-console": "off",
    "no-debugger": "off",
    "no-alert": "error"
  }
};

This will fix your problem. If it does fix your problem it even proves further that we are talking about the same issue :)

(or in other words if you would apply the given workaround your repo as it is now would and should always fail without adapting the .eslintrc)

@gdmoore
Copy link
Author

gdmoore commented May 8, 2020

@gdmoore

So let me try to explain what is happening in your case and clarify my assumption why your issue is the same issue as our issue:

I see, knowing that babel runs after linting leads me to agree that we are talking about the exact same issue. When I build successfully the second build is using the previously-cached results from the first build which have already been transformed to drop the console statement, which now passes linting.

There is still a bug (or mis-feature?) here directly related to console and linting: Directives that modify source, like transform-remove-console, should occur before linting. If they don't, the linting directives can't be relied upon.

I explicitly have the rules set to disallow console statements so that in my real project if someone changes the babel config, or if babel doesn't run properly, and it doesn't strip console statements, I want the build to fail so that I don't ship a release with console statements in it. If I take your advice, it's possible that I could end up shipping with logs.

@activenode
Copy link

Directives that modify source, like transform-remove-console, should occur before linting
Such a differentiation would be probably quite hard to achieve ("shall this run after or before?")
Also it actually makes sense that the code transformer for the dist/output will run after linting because linting is meant to avoid the compilation in the first place if it does not meet your criteria.

So that means

If I take your advice, it's possible that I could end up shipping with logs.

Not if you leave your existing code of .eslintrc because it states with
"no-console": process.env.NODE_ENV === "production" ? "error" : "off",
that it would NOT compile if you try to build with console.log included for production.

tldr: There is other solutions than using babel for removing console.log .

I recommend that you stick with your repo that you have and get errors before compilation when you have console.log in your project (which is why it is so important that THIS github issue gets fixed).

Now you could say: "But I would still want console.log for my development build"

There is multiple different solutions for this problem:

  • Big companies tend to want the console.log on production as well to be able to debug. So they do something like
logText = function() {
   if (window.LOGGING_ACTIVATED) {
      console.log(arguments)
   } // else DO NOT LOG
}
  • You adapt your webpack configuration such as that you activate the babel plugin that you use only for PROD build (so different babelrc per build type). But this would imply that the ESLint would still complain then and prevent you from building PROD. Also here you could adapt the webpack and provide 2 different linting files with a flag.

If I take your advice, it's possible that I could end up shipping with logs.
This also points to the fact that you mistrust babel being successfull which I can understand. So if that is the case I can recommend: Why dont you run eslint on the compiled prod build to make sure there is actually no console statement? Pseudocode: yarn build && eslint dist/myfile.js

But then again. This goes beyond the scope of the issue that is described :)

@JeffJassky
Copy link

JeffJassky commented Sep 20, 2020

Having the same issue. Would certainly like some resolution as this is breaking my CI/CD builds.

An additional workaround is adding --skip-plugins @vue/cli-plugin-eslint to your vue-cli-service serve or vue-cli-service build commands. Be aware that this will skip linting transformations, so use at your own risk.

@akaita
Copy link

akaita commented Nov 3, 2020

Same thing happens to me with a new project... quite unfortunate for Vue, tbh.
I didn't want to mess with project settings to work around a platform bug, as I'm 100% sure those will stay there forever after the bug is solved.
My solution is to just build twice, by ignoring the first try. Sharing my solution for Bitbucket:

- step:
    name: Build
    script:
      - npm install
      - npm run build || true # Ignoring the output of this step because the first build fails https://github.com/vuejs/vue-cli/issues/5399
      - npm run build
    artifacts:
      - dist/**

@OddDev
Copy link

OddDev commented Nov 3, 2020

Same thing happens to me with a new project... quite unfortunate for Vue, tbh.
I didn't want to mess with project settings to work around a platform bug, as I'm 100% sure those will stay there forever after the bug is solved.
My solution is to just build twice, by ignoring the first try. Sharing my solution for Bitbucket:

- step:
    name: Build
    script:
      - npm install
      - npm run build || true # Ignoring the output of this step because the first build fails https://github.com/vuejs/vue-cli/issues/5399
      - npm run build
    artifacts:
      - dist/**

Wait, that produces false positives. The should-fail-build fails correctly in the first build (which is correct) but does not for any additional builds.

@akaita
Copy link

akaita commented Nov 4, 2020

@OddDev npm run build || true always fails to build, but doesn't stop the process.
npm run build succeeds/fails depending on whether it should actually succeed/fail.

@fangbinwei
Copy link
Collaborator

fangbinwei commented Nov 4, 2020

duplicate of #3065

let eslint emit error as warning in production
vue.config.js

/** @type {import('@vue/cli-service').ProjectOptions} */
module.exports = {
  lintOnSave: true
};

@gerbenoostra
Copy link

gerbenoostra commented Nov 16, 2020

Good to know I'm not the only one, still having this issue.
I asked it (quite elaborately, with different attempts) on stackoverflow: https://stackoverflow.com/questions/62497239/cant-remove-console-statements-with-babel-nor-terser-in-vue-cli-3-4-but-se
And didn't get a real answer. For now, I went with answer:

adding lintOnSave: process.env.NODE_ENV === 'development' in vue.config.js.

But it feels tricky, as I don't have any linting on the production build....

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

8 participants