Skip to content

Commit

Permalink
Add #__PURE__ to __jsr.r call in ESM output only, update tests. (#308)
Browse files Browse the repository at this point in the history
* Add #__PURE__ to __jsr.r call in ESM output only, update tests.

* Updated docs on tree shaking

---------

Co-authored-by: Josh Wilson <[email protected]>
  • Loading branch information
joshwilsonvu and Josh Wilson authored Jul 26, 2023
1 parent f5aae39 commit 7f0f91d
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 9 deletions.
20 changes: 16 additions & 4 deletions Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -414,9 +414,14 @@ JsRoutes itself does not have security holes.
It makes URLs without access protection more reachable by potential attacker.
If that is an issue for you, you may use one of the following solutions:

### Explicit Import + ESM Tree shaking
### ESM Tree shaking

Make sure `module_type` is set to `ESM` (the default) and JS files import only required routes into the file like:
Make sure `module_type` is set to `ESM` (the default). Modern JS bundlers like
[Webpack](https://webpack.js.org) can statically determine which ESM exports are used, and remove
the unused exports to reduce bundle size. This is known as [Tree
Shaking](https://webpack.js.org/guides/tree-shaking/).

JS files can use named imports to import only required routes into the file, like:

``` javascript
import {
Expand All @@ -428,8 +433,15 @@ import {
} from '../routes'
```

Such import structure allows for moddern JS bundlers like [Webpack](https://webpack.js.org/) to only include explicitly imported routes into JS bundle file.
See [Tree Shaking](https://webpack.js.org/guides/tree-shaking/) for more information.
JS files can also use star imports (`import * as`) for tree shaking, as long as only explicit property accesses are used.

``` javascript
import * as routes from '../routes';

console.log(routes.inbox_path); // OK, only `inbox_path` is included in the bundle

console.log(Object.keys(routes)); // forces bundler to include all exports, breaking tree shaking
```

### Exclude option

Expand Down
11 changes: 9 additions & 2 deletions lib/js_routes/route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,15 @@ def helper_types
end

def body(absolute)
@configuration.dts? ?
definition_body : "__jsr.r(#{arguments(absolute).map{|a| json(a)}.join(', ')})"
if @configuration.dts?
definition_body
else
# For tree-shaking ESM, add a #__PURE__ comment informing Webpack/minifiers that the call to `__jsr.r`
# has no side-effects (e.g. modifying global variables) and is safe to remove when unused.
# https://webpack.js.org/guides/tree-shaking/#clarifying-tree-shaking-and-sideeffects
pure_comment = @configuration.esm? ? '/*#__PURE__*/ ' : ''
"#{pure_comment}__jsr.r(#{arguments(absolute).map{|a| json(a)}.join(', ')})"
end
end

def definition_body
Expand Down
2 changes: 1 addition & 1 deletion spec/js_routes/module_types/dts_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
describe "compiled javascript asset" do
subject { ERB.new(File.read("app/assets/javascripts/js-routes.js.erb")).result(binding) }
it "should have js routes code" do
is_expected.to include("export const inbox_message_path = __jsr.r(")
is_expected.to include("export const inbox_message_path = /*#__PURE__*/ __jsr.r(")
end
end
end
4 changes: 2 additions & 2 deletions spec/js_routes/module_types/esm_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
* @param {object | undefined} options
* @returns {string} route path
*/
export const inboxes_path = __jsr.r
export const inboxes_path = /*#__PURE__*/ __jsr.r(
DOC
end

Expand All @@ -39,7 +39,7 @@
describe "compiled javascript asset" do
subject { ERB.new(File.read("app/assets/javascripts/js-routes.js.erb")).result(binding) }
it "should have js routes code" do
is_expected.to include("export const inbox_message_path = __jsr.r(")
is_expected.to include("export const inbox_message_path = /*#__PURE__*/ __jsr.r(")
end
end
end

0 comments on commit 7f0f91d

Please sign in to comment.