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

Added TypeScript upgrade docs for Webpacker 5.1 #2541

Merged
merged 12 commits into from
Apr 20, 2020

Conversation

gopeter
Copy link
Contributor

@gopeter gopeter commented Apr 20, 2020

As mentioned in #2449, we need some documentation to upgrade the TypeScript configuration for users who have updated to webpacker >= 5.1

I'll have a look how we can implement this as a rake task too.

@gopeter
Copy link
Contributor Author

gopeter commented Apr 20, 2020

A possible rake task could look like this:

say "Remove old packages"
run "yarn remove ts-loader"

say "Add new packages"
run "yarn add @babel/preset-typescript babel-preset-typescript-vue -D"

say "Remove old configuration files"
remove_file "config/webpack/loaders/typescript.js"

say "Remove old webpack configurations"
gsub_file Rails.root.join("config/webpack/environment.js").to_s, /const typescript =  require\('\.\/loaders\/typescript'\)\n/, ""
gsub_file Rails.root.join("config/webpack/environment.js").to_s, /environment\.loaders\.prepend\('typescript', typescript\)\n/, ""

say "Adding TypeScript preset to babel.config.js"
insert_into_file Rails.root.join("babel.config.js").to_s,
  ",\n      ['@babel/preset-typescript', { 'allExtensions': true, 'isTSX': true }]",
  before: /\s*\].filter\(Boolean\),\n\s*plugins: \[/

But we may have to make Remove old webpack configurations more ... generic (the same applies for the last step also)?
For example: if users have Prettier installed, there could be a space inside the brackets. Or a semicolon at the end. Or double quotes instead of single quotes. And so on ... so we may miss these lines. Is there a way to delete a whole line if the regex matches just a part of it? Searching for /loaders/typescript only should do the trick?

Additionally, should we add a warning to Rails' console if we find old configurations while running version 5.1?

@spiderpug
Copy link

spiderpug commented Apr 20, 2020

I was able to migrate to 5.1 using these instructions, so this works fine.

However, and I'm not sure where to put this (here or #2449), asset compilation in a production environment now fails. This is because the instructions tell me to install @babel/preset-typescript (and/or babel-preset-typescript-vue) as devDependencies. It seems those are not installed when RAILS_ENV=production rails assets:precompile calls yarn install ?

Example stacktrace for `@babel/preset-typescript`
ERROR in ./app/javascript/packs/hello_typescript.ts
Module build failed (from ./node_modules/babel-loader/lib/index.js):
Error: Cannot find module '@babel/preset-typescript' from '/private/tmp/rails-test/TestApp'
    at Function.resolveSync [as sync] (/private/tmp/rails-test/TestApp/node_modules/resolve/lib/sync.js:83:15)
    at resolveStandardizedName (/private/tmp/rails-test/TestApp/node_modules/@babel/core/lib/config/files/plugins.js:101:31)
    at resolvePreset (/private/tmp/rails-test/TestApp/node_modules/@babel/core/lib/config/files/plugins.js:58:10)
    at loadPreset (/private/tmp/rails-test/TestApp/node_modules/@babel/core/lib/config/files/plugins.js:77:20)
    at createDescriptor (/private/tmp/rails-test/TestApp/node_modules/@babel/core/lib/config/config-descriptors.js:154:9)
    at /private/tmp/rails-test/TestApp/node_modules/@babel/core/lib/config/config-descriptors.js:109:50
    at Array.map (<anonymous>)
    at createDescriptors (/private/tmp/rails-test/TestApp/node_modules/@babel/core/lib/config/config-descriptors.js:109:29)
    at createPresetDescriptors (/private/tmp/rails-test/TestApp/node_modules/@babel/core/lib/config/config-descriptors.js:101:10)
    at presets (/private/tmp/rails-test/TestApp/node_modules/@babel/core/lib/config/config-descriptors.js:47:19)
    at mergeChainOpts (/private/tmp/rails-test/TestApp/node_modules/@babel/core/lib/config/config-chain.js:320:26)
    at /private/tmp/rails-test/TestApp/node_modules/@babel/core/lib/config/config-chain.js:283:7
    at Generator.next (<anonymous>)
    at buildRootChain (/private/tmp/rails-test/TestApp/node_modules/@babel/core/lib/config/config-chain.js:90:27)
    at buildRootChain.next (<anonymous>)
    at loadPrivatePartialConfig (/private/tmp/rails-test/TestApp/node_modules/@babel/core/lib/config/partial.js:95:62)
    at loadPrivatePartialConfig.next (<anonymous>)
    at Function.<anonymous> (/private/tmp/rails-test/TestApp/node_modules/@babel/core/lib/config/partial.js:120:25)
    at Generator.next (<anonymous>)
    at evaluateSync (/private/tmp/rails-test/TestApp/node_modules/gensync/index.js:244:28)
    at Function.sync (/private/tmp/rails-test/TestApp/node_modules/gensync/index.js:84:14)
    at Object.<anonymous> (/private/tmp/rails-test/TestApp/node_modules/@babel/core/lib/config/index.js:41:61)
    at Object.<anonymous> (/private/tmp/rails-test/TestApp/node_modules/babel-loader/lib/index.js:151:26)
    at Generator.next (<anonymous>)
    at asyncGeneratorStep (/private/tmp/rails-test/TestApp/node_modules/babel-loader/lib/index.js:3:103)
    at _next (/private/tmp/rails-test/TestApp/node_modules/babel-loader/lib/index.js:5:194)
    at /private/tmp/rails-test/TestApp/node_modules/babel-loader/lib/index.js:5:364
    at new Promise (<anonymous>)
    at Object.<anonymous> (/private/tmp/rails-test/TestApp/node_modules/babel-loader/lib/index.js:5:97)
    at Object.loader (/private/tmp/rails-test/TestApp/node_modules/babel-loader/lib/index.js:64:18)
    at Object.<anonymous> (/private/tmp/rails-test/TestApp/node_modules/babel-loader/lib/index.js:59:12)

@gopeter
Copy link
Contributor Author

gopeter commented Apr 20, 2020

Oh, yes, then this should be installed as a normal dependency – all other loaders and presets aren't devDependencies too.

@gauravtiwari
Copy link
Member

Thanks @gopeter - Please could you link this in changelog under 5.1 release?

@gopeter
Copy link
Contributor Author

gopeter commented Apr 20, 2020

Like so?

## [[5.1.0]](https://github.com/rails/webpacker/compare/v5.0.1...v5.1.0) - 2020-04-19

- Remove yarn integrity check [#2518](https://github.com/rails/webpacker/pull/2518)
- Switch from ts-loader to babel-loader [#2449](https://github.com/rails/webpacker/pull/2449)
- Added upgrade documentation for TypeScript users and fixed yarn dependency errors in production mode [#2541](https://github.com/rails/webpacker/pull/2541)
- Resolve multi-word snakecase WEBPACKER_DEV_SERVER env values [#2528](https://github.com/rails/webpacker/pull/2528)

@shouichi
Copy link

Thank you @gopeter for the work! Followed the instruction and it worked like a charm.

One small question. Do we always need babel-preset-typescript-vue?

  1. Add new packages:
    • yarn add @babel/preset-typescript babel-preset-typescript-vue -D

@gauravtiwari
Copy link
Member

Hmm, maybe just a line beneath,

- Switch from ts-loader to babel-loader [#2449](https://github.com/rails/webpacker/pull/2449)
 Please see the link_to documentation to upgrade existing projects to use typescript with 5.1

Also, babel-preset-typescript-vue, maybe we can make this one optional and include in doc instead of installer.

@gopeter
Copy link
Contributor Author

gopeter commented Apr 20, 2020

Ok, both points are done

@gauravtiwari
Copy link
Member

Great, thanks :)

One last thing, can we use additional packages for vue if defined in package.json, like React? Please see this:

if package["dependencies"].keys.include?("react")

@gauravtiwari
Copy link
Member

It doesn't have to be though, only if it makes sense (since we are doing it for React)

@gopeter
Copy link
Contributor Author

gopeter commented Apr 20, 2020

Hmmm but what happens if someone runs webpacker:install:typescript before webpacker:install:vue? Then babel-preset-typescript-vue would be missing and there wouldn't be a hint about this in the docs.

For React, we're just adding the appropriate types, which can be done by the user too. But since babel-preset-typescript-vue is mandatory to run/compile the code, this should not rely on the right installation-order.

If we want to do this, we would need to add

  if package["dependencies"].keys.include?("typescript")
    additional_packages = "babel-preset-typescript-vue"
  end

to webpacker/lib/install/vue.rb too.

What do you think about this?

@gauravtiwari
Copy link
Member

I see, makes sense. Let's leave it then, maybe we can improve it later.

@gauravtiwari gauravtiwari merged commit f651362 into rails:master Apr 20, 2020
@gauravtiwari
Copy link
Member

Merged, thanks for working on it 🙏

@gopeter
Copy link
Contributor Author

gopeter commented Apr 20, 2020

OK, great! I've created #2543 if you want to have a look.

bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Jan 22, 2021
https://build.opensuse.org/request/show/865216
by user coolo + dimstar_suse
updated to version 5.2.1
 see installed CHANGELOG.md
  ## [[5.2.1]](rails/webpacker@v5.2.0...5.2.1) - 2020-08-17

  - Revert [#1311](rails/webpacker#1311).

  ## [[5.2.0]](rails/webpacker@v5.1.1...5.2.0) - 2020-08-16

  - Bump dependencies and fixes. See [diff](rails/webpacker@v5.1.1...5-x-stable) for changes.

- updated to version 5.1.1
 see installed CHANGELOG.md
  ## [[5.1.1]](rails/webpacker@v5.1.0...v5.1.1) - 2020-04-20

  - Update [TypeScript documentation](https://github.com/rails/webpacker/blob/master/docs/typescript.md) and installer to use babel-loader for typescript.[(#2541](rails/webpacker#2541)

  ## [[5.1.0]](ht
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

Successfully merging this pull request may close these issues.

4 participants