aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* Remove circular dependency warnings in ActionCable javascript and publish ↵rmacklin2018-12-0116-290/+325
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | source modules with fine-grained exports (#34370) * Replace several ActionCable.* references with finer-grained imports This reduces the number of circular dependencies among the module imports from 4: ``` (!) Circular dependency: app/javascript/action_cable/index.js -> app/javascript/action_cable/connection.js -> app/javascript/action_cable/index.js (!) Circular dependency: app/javascript/action_cable/index.js -> app/javascript/action_cable/connection_monitor.js -> app/javascript/action_cable/index.js (!) Circular dependency: app/javascript/action_cable/index.js -> app/javascript/action_cable/consumer.js -> app/javascript/action_cable/index.js (!) Circular dependency: app/javascript/action_cable/index.js -> app/javascript/action_cable/subscriptions.js -> app/javascript/action_cable/index.js ``` to 2: ``` (!) Circular dependency: app/javascript/action_cable/index.js -> app/javascript/action_cable/connection.js -> app/javascript/action_cable/index.js (!) Circular dependency: app/javascript/action_cable/index.js -> app/javascript/action_cable/connection.js -> app/javascript/action_cable/connection_monitor.js -> app/javascript/action_cable/index.js ``` * Remove tests that only test javascript object property assignment These tests really only assert that you can assign a property to the ActionCable global object. That's true for pretty much any object in javascript (it would only be false if the object has been frozen, or has explicitly set some properties to be nonconfigurable). * Refactor ActionCable to provide individual named exports By providing individual named exports rather than a default export which is an object with all of those properties, we enable applications to only import the functions they need: any unused functions will be removed via tree shaking. Additionally, this restructuring removes the remaining circular dependencies by extracting the separate adapters and logger modules, so there are now no warnings when compiling the ActionCable bundle. Note: This produces two small breaking API changes: - The `ActionCable.WebSocket` getter and setter would be moved to `ActionCable.adapters.WebSocket`. If a user is currently configuring this, when upgrading they'd need to either add a delegated getter/setter themselves, or change it like this: ```diff - ActionCable.WebSocket = MyWebSocket + ActionCable.adapters.WebSocket = MyWebSocket ``` Applications which don't change the WebSocket adapter would not need any changes for this when upgrading. - Similarly, the `ActionCable.logger` getter and setter would be moved to `ActionCable.adapters.logger`. If a user is currently configuring this, when upgrading they'd need to either add a delegated getter/setter themselves, or change it like this: ```diff - ActionCable.logger = myLogger + ActionCable.adapters.logger = myLogger ``` Applications which don't change the logger would not need any changes for this when upgrading. These two aspects of the public API have to change because there's no way to export a property setter for `WebSocket` (or `logger`) such that this: ```js import ActionCable from "actioncable" ActionCable.WebSocket = MyWebSocket ``` would actually update `adapters.WebSocket`. (We can only offer that if we have two separate source files like if `index.js` uses `import * as ActionCable from "./action_cable" and then exports a wrapper which has delegated getters and setters for those properties.) This API change is very minor - it should be easy for applications to add the `adapters.` prefix in their assignments or to patch in delegated setters. And especially because most applications in the wild are not ever changing the default value of `ActionCable.WebSocket` or `ActionCable.logger` (because the default values are perfect), this API breakage is worth the tree-shaking benefits we gain. * Include source code in published actioncable npm package This allows actioncable users to ship smaller javascript bundles to visitors using modern browsers, as demonstrated in this repository: https://github.com/rmacklin/actioncable-es2015-build-example In that example, the bundle shrinks by 2.8K (25.2%) when you simply change the actioncable import to point to the untranspiled src. If you go a step further, like this: ``` diff --git a/app/scripts/main.js b/app/scripts/main.js index 17bc031..1a2b2e0 100644 --- a/app/scripts/main.js +++ b/app/scripts/main.js @@ -1,6 +1,6 @@ -import ActionCable from 'actioncable'; +import * as ActionCable from 'actioncable'; let cable = ActionCable.createConsumer('wss://cable.example.com'); cable.subscriptions.create('AppearanceChannel', { ``` then the bundle shrinks by 3.6K (31.7%)! In addition to allowing smaller bundles for those who ship untranspiled code to modern browsers, including the source code in the published package can be useful in other ways: 1. Users can import individual modules rather than the whole library 2. As a result of (1), users can also monkey patch parts of actioncable by importing the relevant module, modifying the exported object, and then importing the rest of actioncable (which would then use the patched object). Note: This is the same enhancement that we made to activestorage in c0368ad090b79c19300a4aa133bb188b2d9ab611 * Remove unused commonjs & resolve plugins from ActionCable rollup config These were added when we copied the rollup config from ActiveStorage, but ActionCable does not have any commonjs dependencies (it doesn't have any external dependencies at all), so these plugins are unnecessary here * Change ActionCable.startDebugging() -> ActionCable.logger.enabled=true and ActionCable.stopDebugging() -> ActionCable.logger.enabled=false This API is simpler and more clearly describes what it does * Change Travis configuration to run yarn install at the root for ActionCable builds This is necessary now that the repository is using Yarn Workspaces
* Merge pull request #34583 from gregmolnar/guides_fixRyuta Kamizono2018-12-011-1/+1
|\ | | | | fix example code syntax [ci skip]
| * fix example code syntax [ci skip]Greg Molnar2018-12-011-1/+1
|/
* Merge pull request #34579 from gmcgibbon/yield_in_with_delivery_jobRyuta Kamizono2018-12-011-0/+1
|\ | | | | Add yield to with_delivery_job test helper
| * Add yield to with_delivery_job test helperGannon McGibbon2018-11-301-0/+1
| | | | | | | | | | Adds yield to parameterized mail test helper so assertions passed into with_delivery_job are actually ran.
* | `metadata` is not passed to serviceyuuji.yaginuma2018-12-011-2/+1
| | | | | | | | | | | | | | | | | | Ref: https://github.com/rails/rails/blob/604fac6d7191fca102380b1a5f5eb9c619fb407f/activestorage/app/models/active_storage/blob.rb#L256-L264 This fixes broken `GCSServiceTest`. https://travis-ci.org/rails/rails/jobs/461868394#L6624-L6626 Follow up to #34576.
* | Merge pull request #34577 from gmcgibbon/id_as_non_pk_docsRyuta Kamizono2018-12-012-5/+8
|\ \ | | | | | | | | | | | | Clarify no support for non PK id columns [ci skip]
| * | Clarify no support for non PK id columnsGannon McGibbon2018-11-302-5/+8
| |/ | | | | | | [ci skip]
* | Merge pull request #33882 from ↵Rafael França2018-11-302-29/+59
|\ \ | | | | | | | | | | | | mberlanda/mberlanda/as-inheritable-options-intialization [Realties] config_for as ActiveSupport::OrderedOptions
| * | chore: implement config_for as ActiveSupport::OrderedOptionsMauro Berlanda2018-10-192-29/+59
| | |
* | | Merge pull request #34576 from ys/test-upload-extra-keysRafael França2018-11-301-0/+19
|\ \ \ | |_|/ |/| | Add a test with extra keys to active_storage Service#upload
| * | Add a test with extra keys to active_storage Service#uploadYannick Schutz2018-11-301-0/+19
| | |
* | | Merge pull request #34505 from eileencodes/add-readonly-modeEileen M. Uchitelle2018-11-3010-1/+286
|\ \ \ | |/ / |/| | Add ability to block writes to a database
| * | Add ability to prevent writes to a databaseEileen Uchitelle2018-11-3010-1/+286
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This PR adds the ability to prevent writes to a database even if the database user is able to write (ie the database is a primary and not a replica). This is useful for a few reasons: 1) when converting your database from a single db to a primary/replica setup - you can fix all the writes on reads early on, 2) when we implement automatic database switching or when an app is manually switching connections this feature can be used to ensure reads are reading and writes are writing. We want to make sure we raise if we ever try to write in read mode, regardless of database type and 3) for local development if you don't want to set up multiple databases but do want to support rw/ro queries. This should be used in conjunction with `connected_to` in write mode. For example: ``` ActiveRecord::Base.connected_to(role: :writing) do Dog.connection.while_preventing_writes do Dog.create! # will raise because we're preventing writes end end ActiveRecord::Base.connected_to(role: :reading) do Dog.connection.while_preventing_writes do Dog.first # will not raise because we're not writing end end ```
* | Merge pull request #34572 from kamipo/fix_scoping_with_query_methodRyuta Kamizono2018-11-303-2/+8
|\ \ | | | | | | Fix the scoping with query methods in the scope block
| * | Fix the scoping with query methods in the scope blockRyuta Kamizono2018-11-303-2/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | Follow up #33394. #33394 only fixes the case of scoping with klass methods in the scope block which invokes `klass.all`. Query methods in the scope block also need to invoke `klass.all` to be affected by the scoping.
* | | Use the full link URL instead of bit.ly [ci skip]Ryuta Kamizono2018-11-301-1/+2
| | |
* | | Don't expose internal `clock_gettime_supported?` class methodRyuta Kamizono2018-11-301-1/+2
| | |
* | | Merge pull request #34569 from gmcgibbon/allow_attribute_aliases_in_updateRafael França2018-11-293-3/+17
|\ \ \ | | | | | | | | Allow aliased attributes in update
| * | | Allow aliased attributes in updateGannon McGibbon2018-11-293-3/+17
| | | | | | | | | | | | | | | | Allow aliased attributes to be used in `#update_columns` and `#update`.
* | | | Merge pull request #34553 from mjtko/fix/issue-14802Rafael França2018-11-294-3/+30
|\ \ \ \ | |/ / / |/| | | Do nothing when the same block is included again
| * | | Do nothing when the same block is included again.Mark J. Titorenko2018-11-294-3/+30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the same block is included multiple times, we no longer raise an exception or overwrite the included block instance variable. Fixes #14802. [Mark J. Titorenko + Vlad Bokov]
* | | | Merge pull request #34564 from toncid/patch-2Arun Agrawal2018-11-291-0/+1
|\ \ \ \ | | | | | | | | | | Add a Delayed Job project link.
| * | | | [ci skip] Add a Delayed Job project link.Tonči Damjanić2018-11-291-0/+1
|/ / / / | | | | | | | | Delayed Job is mentioned multiple times in the document, but it is not linked from anywhere.
* | | | Merge pull request #34562 from ruralocity/active-record-query-docs-improvementRafael França2018-11-281-7/+9
|\ \ \ \ | | | | | | | | | | Improve ActiveRecord::Querying documentation [ci skip]
| * | | | Improve ActiveRecord::Querying documentation [ci skip]Aaron Sumner2018-11-281-7/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * Break up long sentences * Reword some sentences to clarify subject, predicate, and object * Explain drawbacks of using count_by_sql
* | | | | Merge pull request #34554 from sj26/group-exception-logsRafael França2018-11-281-5/+8
|\ \ \ \ \ | | | | | | | | | | | | Log exceptions atomically
| * | | | | Avoid extra array allocationsSamuel Cochran2018-11-291-2/+2
| | | | | |
| * | | | | Log exceptions atomicallySamuel Cochran2018-11-281-5/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When distributed over multiple logger calls the lines can become intermixed with other log statements. Combining them into a single logger call makes sure they always get logged together.
* | | | | | Merge pull request #34561 from gmcgibbon/allow_spaces_in_table_namesRafael França2018-11-283-1/+12
|\ \ \ \ \ \ | |_|/ / / / |/| | | | | Allow spaces in postgres table names
| * | | | | Allow spaces in postgres table namesGannon McGibbon2018-11-283-1/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fixes issue where "user post" is misinterpreted as "\"user\".\"post\"" when quoting table names with the postgres adapter.
* | | | | | Merge pull request #34534 from gmcgibbon/ar_query_scope_body_docRafael França2018-11-281-1/+1
|\ \ \ \ \ \ | | | | | | | | | | | | | | Clarify scope body requirements
| * | | | | | Clarify scope body requirementsGannon McGibbon2018-11-261-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | [ci skip]
* | | | | | | Add autoload hook for AbstractController::ActionNotFoundRafael Mendonça França2018-11-281-0/+1
| |/ / / / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The error can be reproduced with require "bundler/setup" require "action_controller" AbstractController::ActionNotFound
* | | | | | Merge pull request #34557 from sergioisidoro/sergio-patch-load-errorRafael França2018-11-281-2/+2
|\ \ \ \ \ \ | | | | | | | | | | | | | | Patch load error in case GemSpecError
| * | | | | | Patch load error in case GemSpecErrorsergioisidoro2018-11-281-2/+2
| | | | | | |
* | | | | | | Merge pull request #34550 from mogulla3/fix-argument-error-when-uploding-to-s3Eileen M. Uchitelle2018-11-282-1/+5
|\ \ \ \ \ \ \ | |_|_|_|/ / / |/| | | | | | Fix `ArgumentError` when uploading to amazon s3
| * | | | | | Fix `ArgumentError` when uploading to amazon s3Hiroki Sanpei2018-11-282-1/+5
|/ / / / / /
* | | | | | Use `Testing::Parallelization` in Action Packs's testyuuji.yaginuma2018-11-281-77/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Parallel execution of `ForkingExecutor` is the same approach as `Testing::Parallelization`. So do not need to have own code inside Action Pack. Let's use an already existing feature.
* | | | | | Pass the test reporter by referenceyuuji.yaginuma2018-11-281-0/+1
| |_|/ / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This prevents the array from being dumped as a DRbObject so we can reduce communication with the server. In DRb, if `Marshal.dump` fails, `Marshal.dump` is executed again after converting the object to `DRbObject`. This also possible to reduce the execution of `Marshal.dump` by converting to a format that can be marshalized in advance using `DRbObject`. This is the same approach to Action Pack's parallel test. Ref: 5751b7ea58d7cf259dda30fb42fff51fc6ae93d5
* | | | | Fix "warning: ambiguous first argument; put parentheses or a space even ↵yuuji.yaginuma2018-11-281-3/+3
| | | | | | | | | | | | | | | | | | | | after `/' operator"
* | | | | Merge pull request #34546 from y-yagi/fix_ast_buildYuji Yaginuma2018-11-283-17/+3
|\ \ \ \ \ | | | | | | | | | | | | Fix broken ASt build
| * | | | | text is treated as `attachment`yuuji.yaginuma2018-11-281-2/+2
| | | | | |
| * | | | | Fix broken `ActiveStorage::BlobTest`yuuji.yaginuma2018-11-281-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | `ActiveStorage::Filename#parameters` was removed by #33829.
| * | | | | Remove duplicated testyuuji.yaginuma2018-11-281-14/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since 06ab7b27ea1c1ab357085439abacdb464f6742bf, `GCSServiceTest#test_signed_URL_response_headers` is broken. https://travis-ci.org/rails/rails/jobs/460454477#L7084-L7087 This seems to be due to lack of `content_type` at upload. This is solved by specifying `conten_type`. However, since the same content is also tested with `test_upload_with_content_type`, it will be duplicated content, so I think that can remove `test_signed_URL_response_headers`.
* | | | | | Merge pull request #33835 from schneems/schneems/faster_cache_versionSean Griffin2018-11-272-4/+130
|\ \ \ \ \ \ | |/ / / / / |/| | | | | Use raw time string from DB to generate ActiveRecord#cache_version
| * | | | | Prefer String#ljust over String#<< for paddinglsylvester2018-10-171-3/+2
| | | | | |
| * | | | | Do not silently fail to generate a cache_versionschneems2018-10-172-6/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When an `updated_at` column exists on the model, but is not available on the instance (likely due to a select), we should raise an error rather than silently not generating a cache_version. Without this behavior it's likely that cache entries will not be able to be invalidated and this will happen without notice. This behavior was reported and described by @lsylvester in https://github.com/rails/rails/pull/34197#issuecomment-429668759.
| * | | | | Use raw time string from DB to generate ActiveRecord#cache_versionschneems2018-10-172-4/+118
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, the `updated_at` field is used to generate a `cache_version`. Some database adapters return this timestamp value as a string that must then be converted to a Time value. This process requires a lot of memory and even more CPU time. In the case where this value is only being used for a cache version, we can skip the Time conversion by using the string value directly. - This PR preserves existing cache format by converting a UTC string from the database to `:usec` format. - Some databases return an already converted Time object, in those instances, we can directly use `created_at`. - The `updated_at_before_type_cast` can be a value that comes from either the database or the user. We only want to optimize the case where it is from the database. - If the format of the cache version has been changed, we cannot apply this optimization, and it is skipped. - If the format of the time in the database is not UTC, then we cannot use this optimization, and it is skipped. Some databases (notably PostgreSQL) returns a variable length nanosecond value in the time string. If the value ends in a zero, then it is truncated For instance instead of `2018-10-12 05:00:00.000000` the value `2018-10-12 05:00:00` is returned. We detect this case and pad the remaining zeros to ensure consistent cache version generation. Before: Total allocated: 743842 bytes (6626 objects) After: Total allocated: 702955 bytes (6063 objects) (743842 - 702955) / 743842.0 # => 5.4% ⚡️⚡️⚡️⚡️⚡️ Using the CodeTriage application and derailed benchmarks this PR shows between 9-11% (statistically significant) performance improvement versus the commit before it. Special thanks to @lsylvester for helping to figure out a way to preserve the usec format and for helping with many implementation details.
* | | | | | Merge pull request #34544 from ahawrylak/fix-active-storage-docs-typoGannon McGibbon2018-11-271-1/+1
|\ \ \ \ \ \ | | | | | | | | | | | | | | Fix minor Active Storage docs typo [ci skip]
generated by cgit v1.2.3 (git 2.25.1) at 2024-06-26 00:58:59 +0000