Skip to content

Commit

Permalink
fix package.json browser overrides for nested files
Browse files Browse the repository at this point in the history
fixes #689
  • Loading branch information
thheller committed Apr 25, 2020
1 parent d3539a1 commit 3d33116
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 12 deletions.
39 changes: 27 additions & 12 deletions src/main/shadow/build/npm.clj
Expand Up @@ -343,8 +343,31 @@

(defn find-file
[npm ^File require-from ^String require]
(let [use-browser-overrides (get-in npm [:js-options :use-browser-overrides])]
(let [use-browser-overrides
(get-in npm [:js-options :use-browser-overrides])

require-from-pkg
(when require-from ;; no overrides for entries
(find-package-for-file npm require-from))

browser-override
(when (and use-browser-overrides require-from-pkg)
(get-in require-from-pkg [:browser-overrides require]))]

(cond
;; browser override { "./foo" : false } to signal ignoring this dep
(false? browser-override)
false

;; if package.json has browser: { "./foo" : "./foo.browser" } then all
;; requires in that package using require("./foo") apparantely should be using
;; ./foo.browser instead. regardless of which directory they are in.
;; this seems to be in addition to overriding files with an package-relative path after
;; they have been resolved.
;; if we have a match then just continue resolving that directly in place of the original
(seq browser-override)
(find-file npm require-from browser-override)

(util/is-absolute? require)
(throw (ex-info "absolute require not allowed for node_modules files"
{:tag ::absolute-path
Expand All @@ -356,19 +379,11 @@
(find-relative npm require-from require)

:else
(let [require-from-pkg
(when require-from ;; no overrides for entries
(find-package-for-file npm require-from))

browser-override
(and use-browser-overrides
require-from-pkg
(get-in require-from-pkg [:browser-overrides require]))

override
(when use-browser-overrides
(let [override
(when (and use-browser-overrides browser-override)
(if (some? browser-override)
browser-override
;; potentially override require("fs") to the browser polyfill for that
(get node-libs-browser require)))]

(cond
Expand Down
16 changes: 16 additions & 0 deletions src/test/shadow/build/npm_test.clj
Expand Up @@ -45,6 +45,22 @@
(is (= "node_modules/pkg-a/index-browser.js" resource-name))
)))

(deftest test-nested-browser-override
(with-npm [x {}]
(let [{:keys [file] :as require-from-rc}
(find-npm-resource x nil "pkg-nested-override/dir/bar")

;; emulate require("./foo") from bar.js
{:keys [resource-name ns file package-name] :as rc}
(find-npm-resource x file "./foo")]

(is rc)
(is (string? resource-name))
(is (= 'module$node_modules$pkg_nested_override$dir$foo_browser ns))
(is (= "node_modules/pkg-nested-override/dir/foo.browser.js" resource-name))
)))


(deftest test-no-browser-override
(with-npm [x {}]
(let [{:keys [resource-name ns file package-name] :as rc}
Expand Down
1 change: 1 addition & 0 deletions test-env/pkg-nested-override/dir/bar.js
@@ -0,0 +1 @@
require("./foo")
1 change: 1 addition & 0 deletions test-env/pkg-nested-override/dir/foo.browser.js
@@ -0,0 +1 @@
module.exports = "ok";
1 change: 1 addition & 0 deletions test-env/pkg-nested-override/dir/foo.js
@@ -0,0 +1 @@
SHOULD NOT BE INCLUDED!
1 change: 1 addition & 0 deletions test-env/pkg-nested-override/index.js
@@ -0,0 +1 @@
require("./dir/bar.js")
9 changes: 9 additions & 0 deletions test-env/pkg-nested-override/package.json
@@ -0,0 +1,9 @@
{
"name": "pkg-nested-override",
"version": "1.0.0",
"description": "",
"main": "index.js",
"browser":{
"./foo":"./foo.browser"
}
}

0 comments on commit 3d33116

Please sign in to comment.