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

With ts-loader, files aren't correctly cached if there is a compilation error. This leads to too many or too few calls to compile. #2113

Closed
Taytay opened this issue May 31, 2019 · 10 comments · Fixed by #2117

Comments

@Taytay
Copy link
Contributor

Taytay commented May 31, 2019

Webpacker 4.0.2 with ts-loader 6.0.1.

Summary: The cache file: tmp/cache/webpacker/last-compilation-digest-development is only updated if webpack compilation succeeds. However, even though ts-loader fails the webpack build if there is a Typescript error, webpack still emits an updated manifest.json and an output .js file. So, if you introduce an error in your source Typescript file and the webpack compiler runs, the cache file will still indicate that the output is based on the contents of the file BEFORE the error was introduced. So, if you then undo your change to the source file and reload the page, webpack is NOT invoked as it should be.

Furthermore, this means that as long as there is an error in any Typescript file, webpack will ignore the cache and will ALWAYS invoke compilation, causing repeated compilations of the exact same files with the exact same source.

Details:

Not using webpack dev server, but compiling on demand, meaning that it should only compile if there is a change.

I have a file in app/javascript/packs/authentications.ts that compiles correctly.
I load my login page and see that Webpack is compiling, as I expect:

web_1       | 18:07:28 web.1               | [Webpacker] Compiling…
web_1       | 18:07:39 web.1               | [Webpacker] Compiled all packs in /app/ynab_api/public/assets/packs
web_1       | 18:07:39 web.1               |   Rendered authentications/new.html.erb within layouts/application (10567.7ms)
web_1       | 18:07:39 web.1               |   Rendered shared/_favicon.erb (20.3ms)
web_1       | 18:07:39 web.1               | Completed 200 OK in 10919ms (Views: 10904.9ms | ActiveRecord: 0.0ms)

I look at my cache file and see:

$ cat tmp/cache/webpacker/last-compilation-digest-development
66a64d57cdc91eabf692e2bc6318c555fa6e10b2%

So far, so good. Now, introduce a compile error and reload the page.
Again, it compiles, but this time it displays a compilation error. So far, so good:

web_1       | 18:09:25 web.1               | [Webpacker] Compiling…
web_1       | 18:09:35 web.1               | [Webpacker] Compilation failed:
web_1       | 18:09:35 web.1               |
web_1       | 18:09:35 web.1               | Hash: 63e8d239697434693810
web_1       | 18:09:35 web.1               | Version: webpack 4.29.6
web_1       | 18:09:35 web.1               | Time: 8590ms
web_1       | 18:09:35 web.1               | Built at: 2019-05-31 18:09:35
web_1       | 18:09:35 web.1               |                                      Asset       Size           Chunks             Chunk Names
web_1       | 18:09:35 web.1               | js/authentications-f513c39af604d8951a62.js    377 KiB  authentications  [emitted]  authentications
web_1       | 18:09:35 web.1               |            js/otps-822bdb1661898df52846.js   5.48 KiB             otps  [emitted]  otps
web_1       | 18:09:35 web.1               |                              manifest.json  391 bytes                   [emitted]
web_1       | 18:09:35 web.1               | Entrypoint authentications = js/authentications-f513c39af604d8951a62.js
web_1       | 18:09:35 web.1               | Entrypoint otps = js/otps-822bdb1661898df52846.js
web_1       | 18:09:35 web.1               | [./app/javascript/packs/authentications.ts] 39 KiB {authentications} [built] [1 error]
web_1       | 18:09:35 web.1               | [./app/javascript/packs/otps.coffee] 1.65 KiB {otps} [built]
web_1       | 18:09:35 web.1               | [./node_modules/webpack/buildin/global.js] (webpack)/buildin/global.js 878 bytes {authentications} [built]
web_1       | 18:09:35 web.1               |     + 5 hidden modules
web_1       | 18:09:35 web.1               |
web_1       | 18:09:35 web.1               | ERROR in /app/ynab_api/app/javascript/packs/authentications.ts
web_1       | 18:09:35 web.1               | ./app/javascript/packs/authentications.ts
web_1       | 18:09:35 web.1               | [tsl] ERROR in /app/ynab_api/app/javascript/packs/authentications.ts(6,1)
web_1       | 18:09:35 web.1               |       TS2304: Cannot find name 'asdfsfsd'.

But now the output .js file and manifest.json HAS been updated, but the cache file has NOT been updated!

$ cat tmp/cache/webpacker/last-compilation-digest-development
66a64d57cdc91eabf692e2bc6318c555fa6e10b2%

So now, if I undo my change to the source file, restoring the source authentications.ts file to the state it was in before the error and reload the page, I do NOT see Webpack compile my file.

And of course, the manifest.json and output .js files remain the same, meaning that the compiled file loaded is the one compiled based on the error-containing source file.

@jakeNiemiec
Copy link
Member

jakeNiemiec commented May 31, 2019

Could it be related to that error message?
🙄 That's what I get for skimming.

@Taytay
Copy link
Contributor Author

Taytay commented May 31, 2019

Hey @jakeNiemiec : Thanks for the response. That was an intentionally introduced error in my Typescript. My point is that any error in the Typescript will cause the cache file and the output to get out of sync, meaning that compilations don't happen when they should, or that compilations happen repeatedly, as long as there is an error. It is common in development to have compilation errors, and then to fix them, but with this bug, fixing the error might mean that the source doesn't get recompiled when it should. It might silently skip compilation, leading to a very-difficult-to-track-down bug. :)

@jakeNiemiec
Copy link
Member

After some more sleuthing, I think this is the cause of the problem: #1764

# lib/webpacker/compiler.rb:18
  def compile
    if stale?
      run_webpack.tap do |success|
        record_compilation_digest if success
      end
    else
      true
    end
  end

  # Returns true if all the compiled packs are up to date with the underlying asset files.
  def fresh?
    watched_files_digest == last_compilation_digest
  end

  # Returns true if the compiled packs are out of date with the underlying asset files.
  def stale?
    !fresh?
  end

I think I have a good idea of what you mean, record_compilation_digest might need to run regardless of success, but I can't tell if #1764 would regress if we did that. Could you create a PR that works with your situation?

@Taytay
Copy link
Contributor Author

Taytay commented May 31, 2019

Thanks! In looking briefly at #1764, it looks like that did two things. One of them is to make sure that the cache file isn't written to until AFTER webpack is finished. Unfortunately, it also only wrote to the cache file if webpack succeeded. I'll try always writing to the cache file, regardless of whether or not webpack succeeded, but only after it's done, and let you know how it works. Does that sound right to you?

@jakeNiemiec
Copy link
Member

Yes, if you make a PR that works with your TS, I can test the fork against #1764.

@jakeNiemiec
Copy link
Member

I guess the WDS overlay never worked even though it was enabled by default in the webpacker.yml file.

This is probobly the reason that webpack still spits out a build when there is an error. Ideally, you should get something like this in your browser:
image

I am fairly certain that removing if success will fix your problem and unblock this feature.

@Taytay
Copy link
Contributor Author

Taytay commented Jun 3, 2019

@jakeNiemiec - actually, when I used the WDS, I did see the overlay! If I don't use the WDS though, it compiles on demand, and that's when I see this issue.

@Taytay
Copy link
Contributor Author

Taytay commented Jun 3, 2019

@jakeNiemiec : I made this change in my own local Gem and confirmed it worked. (The digest was always updated, whether i had an error or not. Furthermore, I didn't get continuous recompilations in the case of a Typescript error, and after I undid the changes that led to the error, I correctly got a recompilation as expected). I was not able to test against #1764, as I am not sure what those exact circumstances were. Based on the description, I would think that tapping Webpack and writing the digest in that block would indeed only happen AFTER Webpack runs.

@jakeNiemiec
Copy link
Member

I'll wait a few days for the other dev to respond. In the meantime, I'll kick the tires to see that everything still works.

Thanks for your contribution.

jakeNiemiec added a commit to Taytay/webpacker that referenced this issue Jun 7, 2019
If it returns stale, the error message will never make it to the `webpack-dev-server` overlay. This also fixes typescript related problems in rails#2113
gauravtiwari pushed a commit that referenced this issue Jun 10, 2019
* Record the compilation digest even on webpack error

Fixes /issues/2113

* fix: failed builds should return as fresh

If it returns stale, the error message will never make it to the `webpack-dev-server` overlay. This also fixes typescript related problems in #2113
@IlkhamGaysin
Copy link

IlkhamGaysin commented Mar 13, 2020

Guys, I had memory allocation error during deployment as the digest was set the redeploy didn't compiled assets. I think it's a bug as I expect the digest to be the same as there wasn't any change of assets! Do you need config files from me of my rails app? I have typescript and ts-loader
Fixing one case it's introduced another case...

image

// package.json
{
  "name": "app",
  "private": true,
  "dependencies": {
    "@nivo/bar": "^0.61.1",
    "@rails/webpacker": "4.1.0",
    "@types/ramda": "^0.26.38",
    "@types/react": "^16.9.2",
    "@types/react-dates": "^17.1.7",
    "@types/react-dom": "^16.9.0",
    "@types/react-redux": "^7.1.7",
    "@types/reactstrap": "^6.4.4",
    "actioncable": "^5.2.2",
    "activestorage": "^5.2.2",
    "bootstrap": "4.1.1",
    "caniuse-lite": "^1.0.30000697",
    "coffee-loader": "^0.9.0",
    "coffeescript": "2.0.1",
    "datatables.net": "^1.10.20",
    "datatables.net-bs4": "^1.10.20",
    "deox": "^3.2.1",
    "font-awesome": "^4.7.0",
    "foundation-emails": "^2.2.1",
    "imports-loader": "^0.8.0",
    "jquery": "3.3.1",
    "moment": "^2.24.0",
    "popper.js": "^1.14.6",
    "prop-types": "^15.6.2",
    "rails-ujs": "^5.2.3",
    "ramda": "^0.26.1",
    "react": "^16.9.0",
    "react-dates": "^21.7.0",
    "react-dnd": "^10.0.2",
    "react-dnd-html5-backend": "^10.0.2",
    "react-dom": "^16.9.0",
    "react-redux": "^7.1.3",
    "react-redux-hooks": "^0.3.0",
    "reactstrap": "^7.1.0",
    "redux": "^4.0.5",
    "resolve-url-loader": "^3.1.1",
    "rxjs": "^6.5.4",
    "ts-loader": "^4.5.0",
    "typescript": "^3.8.2",
    "video.js": "^7.2.3",
    "webpack": "4.41.2",
    "webpack-cli": "^3.3.11",
    "whatwg-fetch": "^3.0.0"
  },
  "devDependencies": {
    "@testing-library/jest-dom": "^4.1.0",
    "@testing-library/react": "^9.1.4",
    "@testing-library/react-hooks": "^3.2.1",
    "@types/jest": "^24.0.18",
    "@types/node": "^12.7.4",
    "babel-eslint": "^9.0.0",
    "babel-loader": "^8.0.6",
    "css-loader": "^3.2.0",
    "eslint": "^5.10.0",
    "eslint-config-airbnb-base": "^13.1.0",
    "eslint-config-prettier": "^3.3.0",
    "eslint-import-resolver-webpack": "^0.10.1",
    "eslint-plugin-import": "^2.14.0",
    "eslint-plugin-prettier": "^3.0.0",
    "fetch-mock": "^8.0.0",
    "file-loader": "^4.2.0",
    "jest": "^24.9.0",
    "jest-css-modules": "^2.1.0",
    "jest-extended": "^0.11.2",
    "jest-fetch-mock": "^2.1.2",
    "prettier": "^1.15.3",
    "react-test-renderer": "^16.12.0",
    "react-with-direction": "^1.3.1",
    "sass-loader": "7.3.1",
    "style-loader": "^1.0.0",
    "ts-jest": "^24.0.2",
    "tslib": "^1.9.0",
    "tslint": "^5.12.1",
    "tslint-config-prettier": "^1.17.0",
    "tslint-config-standard": "^8.0.1",
    "tslint-eslint-rules": "^5.4.0",
    "tslint-react": "^3.6.0",
    "webpack-dev-server": "3.10.3"
  },
  "scripts": {
    "prettier:write": "bin/yarn prettier --write 'app/frontend/**/*.{ts,tsx,js,jsx}'",
    "test": "jest --verbose --expand",
    "test:coverage": "jest --coverage"
  }
}
// ts-config.json
{
  "compilerOptions": {
    "declaration": false,
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "lib": ["es7", "dom", "esnext", "es2015"],
    "module": "es6",
    "moduleResolution": "node",
    "sourceMap": true,
    "target": "es5",
    "jsx": "react",
    "allowSyntheticDefaultImports": true,
    "noImplicitAny": true,
    "allowJs": true,
    "strict": false,
    "downlevelIteration": true,
    "esModuleInterop": true,
    "noUnusedLocals": true
  },
  "exclude": [
    "app/frontend/tests/",
    "node_modules",
    "vendor",
    "public"
  ],
  "compileOnSave": false
}
# config/webpacker.yml

default: &default
  source_path: app/frontend
  source_entry_path: packs
  public_root_path: public
  public_output_path: assets/packs
  cache_path: tmp/cache/webpacker
  check_yarn_integrity: true
  webpack_compile_output: true

  # Additional paths webpack should lookup modules
  # ['app/assets', 'engine/foo/app/assets']
  resolved_paths: []

  # Reload manifest.json on all requests so we reload latest compiled packs
  cache_manifest: false

  # Extract and emit a css file
  extract_css: true

  static_assets_extensions:
    - .jpg
    - .jpeg
    - .png
    - .gif
    - .tiff
    - .ico
    - .svg
    - .eot
    - .otf
    - .ttf
    - .woff
    - .woff2

  extensions:
    - .jsx
    - .tsx
    - .ts
    - .js
    - .sass
    - .scss
    - .css
    - .module.sass
    - .module.scss
    - .module.css
    - .png
    - .svg
    - .gif
    - .jpeg
    - .jpg
    - .coffee

development:
  <<: *default
  compile: true

  # Reference: https://webpack.js.org/configuration/dev-server/
  dev_server:
    https: false
    host: localhost
    port: 3035
    public: localhost:3035
    hmr: false
    # Inline should be set to true if using HMR
    inline: true
    overlay: true
    compress: true
    disable_host_check: true
    use_local_ip: false
    quiet: false
    headers:
      'Access-Control-Allow-Origin': '*'
    watch_options:
      ignored: '**/node_modules/**'

test:
  <<: *default
  compile: true

  # Compile test packs to a separate directory
  public_output_path: packs-test

staging:
  <<: *default

  # Staging depends on precompilation of packs prior to booting for performance.
  compile: false

  # Cache manifest.json for performance
  cache_manifest: true

production:
  <<: *default

  # Production depends on precompilation of packs prior to booting for performance.
  compile: false

  # Cache manifest.json for performance
  cache_manifest: true

KingTiger001 added a commit to KingTiger001/Rails-web-pack-project that referenced this issue Jan 15, 2023
* Record the compilation digest even on webpack error

Fixes rails/webpacker/issues/2113

* fix: failed builds should return as fresh

If it returns stale, the error message will never make it to the `webpack-dev-server` overlay. This also fixes typescript related problems in #2113
smartech7 pushed a commit to smartech7/ruby-webpacker that referenced this issue Aug 4, 2023
* Record the compilation digest even on webpack error

Fixes rails/webpacker/issues/2113

* fix: failed builds should return as fresh

If it returns stale, the error message will never make it to the `webpack-dev-server` overlay. This also fixes typescript related problems in #2113
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants