Improve js and css options of app generator

[javierav]

Motivation / Background

This Pull Request has been created because the Rails app generator currently has two issues:

First issue

If I choose importmap as the javascript option and bootstrap, bulma or postcss as the css option, rails forces me to use esbuild as the javascript option, ignoring my choice of importmap.

There should be no problem using importmap as a javascript option and using bootstrap, bulma or postcss with Node.js

Second issue

If I choose importmap as the javascript option, the .node-version file is not created and the Dockerfile does not include the Node.js installation steps, even when Rails forces it to use esbuild (first issue) or if we are using a css option that requires Node.js

Detail

This Pull Request changes the requirements for the .node-version file to be created and the Dockerfile to include the Node.js installation.
It also changes the old behavior and if I choose importmap it uses it regardless of the css option selected.

Actual behavior

options[:javascript]	options[:css]	gems	install commands	require node	install node	status
importmap	bootstrap, bulma, postcss	jsbundling-rails cssbundling-rails	javascript:install:esbuild css:install:*	YES	NO	bug1
importmap	tailwind	importmap-rails tailwindcss-rails	importmap:install tailwindcss:install	NO	NO	ok
importmap	sass	importmap-rails dartsass-rails	importmap:install dartsass:install	NO	NO	ok
webpack, esbuild, rollup	*	jsbundling-rails cssbundling-rails	javascript:install:* css:install:*	YES	YES	ok

Proposed behavior

options[:javascript]	options[:css]	gems	install commands	install node
importmap	bootstrap, bulma, postcss	importmap-rails cssbundling-rails	importmap:install css:install:*	YES
importmap	tailwind	importmap-rails tailwindcss-rails	importmap:install tailwindcss:install	NO
importmap	sass	importmap-rails dartsass-rails	importmap:install dartsass:install	NO
webpack, esbuild, rollup	*	jsbundling-rails cssbundling-rails	javascript:install:* css:install:*	YES

Still, there is a combination that is not possible: if I want to use importmap and cssbundling-rails to compile the css using tailwind or sass instead of use tailwindcss-rails or dartsass-rails.

Checklist

Before submitting the PR make sure the following are checked:

• This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
• Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
• Tests are added or updated if you fix a bug or add a feature.
• CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

Footnotes

1. no install node (required by jsbundling-rails) and change importmap to esbuild ↩

[jonathanhefner]

This makes sense to me, but it would be good to add some tests.

I am pushing a commit to add test coverage and address my comments. Let me know what you think.

[jonathanhefner]

options[:javascript] does have a default value ("importmap"), but specifying --no-javascript would set it to nil, so let's keep the presence check.

[javierav]

Good catch!

[jonathanhefner]

I think using_node? is the correct condition here, and it makes the intent more clear. Is there a reason to not use it?

[javierav]

Look good for me! I have used form options[:javascript] == "importmap" because it has been a literal translation of the PR table, but it seems to me a successful change.

[jonathanhefner]

Thank you, @javierav! 📦