Add backward compatibility by ahangarha · Pull Request #252 · shakacode/shakapacker

ahangarha · 2023-02-18T16:51:25Z

Some points:

Though I ensure to see a deprecation message in some tests in the output, not all the cases are covered.
Some features are not covered by our tests. So they are not covered in these tests (as of the time of writing this description)
I have manually tested backward compatibility on react_on_rails_demo_ssr_hmr project.

vaukalak · 2023-02-24T07:50:12Z

Otherwise LGTM

justin808 · 2023-02-24T21:16:17Z

+    strategy:
+      matrix:
+        os: [ubuntu-latest]
+        ruby: ['2.6', '2.7', '3.0']


If you are okay with updating our test environment in this PR, I can do it. But also we can do it in a separate PR.

My proposal:
Ruby:

- ruby: ['2.6', '2.7', '3.0'] + ruby: ['2.7', '3.0', '3.2']

Node:

- node: [12.x, 14.x, 16.x] + node: [14.x, 16.x, 18.x]

@justin808
What about this?

Maybe better to discuss this for v7.

justin808 · 2023-02-24T21:49:27Z

@@ -0,0 +1,24 @@
+require_relative "test_app/config/environment"


name needs to be more descriptive, maybe with some comments at the beginning of this file

why is this code separate from spec_helper.rb?

Could we run this code with a before_each? this file got required in many different files.

I am afraid we can do this unless we want to initialize a new Rails.application for each single test case, which is expensive.

This would be a temporary implementation till Version 7. From version 7 and dropping backward compatibility, we go back to what we used to be.

I agree about the filename. The file initializes a Rails application and sets Shakapacker or Webpacker configuration. Maybe app_initializer would be a better name. I am afraid if I give it such a name and leave the file in the same directory as other spec files, we cannot recognize it easily. Maybe put it in a helper directory? Not sure of the best approach.

justin808 · 2023-03-08T04:19:24Z

Ready?

justin808 · 2023-03-10T21:08:06Z

+
+  def get_config_file_path_with_backward_compatibility(config_path)
+    if config_path.to_s.end_with?("shakapacker.yml") && !File.exist?(config_path)
+      webpacker_config_path = if config_path.class == Pathname


not formatted correctly

Rubocop forces me to use such indentation. We have this rule:

Use 2 (not 26) spaces for indentation. (convention:Layout/IndentationWidth)

justin808 · 2023-03-14T22:48:57Z

+
+    # For backward compatibility
+    Shakapacker.set_shakapacker_env_variables_for_backward_compatibility
+    @config_path = Pathname.new(ENV["SHAKAPACKER_CONFIG"] || config_path)


was this done somewhere else in a prior PR?

Which line?

If 17, no.

For line no.18, I found it challenging to set config path from env when we do it in Instance class. This way, I let the configuration class make the final decision about the config path.

Perhaps it was better to do it in the renaming PR. Yet, this is just internal change.

* Use Shakapacker module, config filesm, and env variables * Rename rake files to shakapacker * Update JS code and tests * Fix conflict * Update Readme and contribution guide for shakapacker * Remove debug lines * Fix linter issue * Use Shakapacker module, config filesm, and env variables * Add support for old configfile * Add rake tasks * Add files to support requiring Webpacker module * Add backward compatibility test * Add rake task for backward compatibility test * Add CI for backward compatibility specs * Add backward compatibility to JS package * Add backward compatibility for --debug-shakapacker option * Fix chdir in afterAll hook in js tests * Fix js tests for compiling files in source_path * Use symlink address for config file * Use fs.existsSync to check if file exist * Remove a redundant assignment * Improve deprecation messages * Improve gitignore in test apps * Improve deprecation message for env variables * Update documentation * Show deprecation message for old binstubs * Update changelog * Add and improve test for custom config path in env variable * Improve robustness in config.fetch method * Update docs * Update deprecation message * Simplify config.fetch method * Remove lines releated to git conflict * Improve the order of code * Add v7 upgrade guide link to changelog

* Rename Webpacker instances to shakapacker (#245) * Add backward compatibility (#252) * Update docs for spelling change (#263) * Fix backward compatibility for setting WEBPACKER_CONFIG in webpack.config.js (#266) * Remove redundant code for enhancing precompile task (#270) * Update webpack dev server configuration (#276) * Stop stripping top level dirs (#283) * Improve source_entry_path robustness (#284) --------- Co-authored-by: Tom Dracz <[email protected]>

ahangarha changed the title ~~WIP - add backward compatibility~~ WIP - Add backward compatibility Feb 18, 2023

ahangarha force-pushed the add-backward-compatibility branch from 241c00f to df64a02 Compare February 20, 2023 16:04

vaukalak reviewed Feb 24, 2023

View reviewed changes

Comment thread lib/shakapacker/configuration.rb Outdated

ahangarha force-pushed the add-backward-compatibility branch 2 times, most recently from b34a1f6 to 22a9a63 Compare February 24, 2023 18:03

justin808 requested changes Feb 24, 2023

View reviewed changes

ahangarha force-pushed the add-backward-compatibility branch 2 times, most recently from 4dde39b to 255c291 Compare February 28, 2023 10:39

ahangarha marked this pull request as ready for review February 28, 2023 15:46

ahangarha changed the title ~~WIP - Add backward compatibility~~ Add backward compatibility Feb 28, 2023

ahangarha force-pushed the webpacker-to-shakapacker branch from 2776578 to 60ccf1c Compare March 7, 2023 09:56

ahangarha force-pushed the add-backward-compatibility branch 2 times, most recently from a197740 to 2b9440e Compare March 7, 2023 11:42

justin808 mentioned this pull request Mar 8, 2023

Switch away from Webpacker to Shakapacker decidim/decidim#10389

Merged

ahangarha force-pushed the webpacker-to-shakapacker branch from 756e056 to 4cc8b9f Compare March 8, 2023 09:08

ahangarha force-pushed the add-backward-compatibility branch from 2b9440e to d4fc297 Compare March 8, 2023 09:19

justin808 requested changes Mar 10, 2023

View reviewed changes

ahangarha force-pushed the add-backward-compatibility branch from 09d3ee7 to 80636f6 Compare March 12, 2023 14:00

Base automatically changed from webpacker-to-shakapacker to shakapacker-v6.7 March 13, 2023 09:00

ahangarha added 9 commits March 13, 2023 23:04

Use Shakapacker module, config filesm, and env variables

5158e05

Rename rake files to shakapacker

e0461ac

Update JS code and tests

3d317be

Fix conflict

af679e2

Update Readme and contribution guide for shakapacker

133170a

Remove debug lines

2930fff

Fix linter issue

d71c06a

Use Shakapacker module, config filesm, and env variables

3b369fa

Add support for old configfile

688256e

ahangarha added 20 commits March 13, 2023 23:09

Add rake task for backward compatibility test

92c2db6

Add CI for backward compatibility specs

6871252

Add backward compatibility to JS package

70ee6f1

Add backward compatibility for --debug-shakapacker option

61454bd

Fix chdir in afterAll hook in js tests

452c37c

Fix js tests for compiling files in source_path

1ec664f

Use symlink address for config file

54b2802

Use fs.existsSync to check if file exist

c8b66ab

Remove a redundant assignment

068759a

Improve deprecation messages

905e12e

Improve gitignore in test apps

c3788d7

Improve deprecation message for env variables

8144e0b

Update documentation

6a8099a

Show deprecation message for old binstubs

5e12431

Update changelog

251304e

Add and improve test for custom config path in env variable

2a79635

Improve robustness in config.fetch method

76ac368

Update docs

e1e3d90

Update deprecation message

1d3862f

Simplify config.fetch method

fca217a

ahangarha force-pushed the add-backward-compatibility branch from 8a3a0b6 to fca217a Compare March 13, 2023 19:40

Remove lines releated to git conflict

558b72f

justin808 requested changes Mar 14, 2023

View reviewed changes

ahangarha added 2 commits March 15, 2023 23:38

Improve the order of code

5deb523

Add v7 upgrade guide link to changelog

ef150f6

justin808 approved these changes Mar 17, 2023

View reviewed changes

justin808 merged commit a4a15f3 into shakapacker-v6.7 Mar 17, 2023

justin808 deleted the add-backward-compatibility branch March 17, 2023 07:09

		@@ -0,0 +1,24 @@
		require_relative "test_app/config/environment"

Uh oh!

Conversation

ahangarha commented Feb 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

vaukalak commented Feb 24, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahangarha Feb 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahangarha Mar 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justin808 commented Mar 8, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ahangarha commented Feb 18, 2023 •

edited

Loading

ahangarha Feb 28, 2023 •

edited

Loading

ahangarha Mar 11, 2023 •

edited

Loading