Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[plugin] Plugin dependencies can be unexpectedly overridden by thin-hook dependencies (generalized #402) #406

Open
t2ym opened this issue Nov 9, 2020 · 0 comments

Comments

@t2ym
Copy link
Owner

t2ym commented Nov 9, 2020

[plugin] Plugin dependencies can be unexpectedly overridden by thin-hook dependencies (generalized #402)

Root Cause

  • Node.js always expands symlinks in resolving module paths
  • Paths for scoped file:plugins/{pluginName} dependencies are resolved from the paths under node_modules/thin-hook/
  • Dependencies under node_modules/thin-hook/node_modules are unexpectedly loaded for scoped node_modules/@thin-hook/{pluginName}

Status - Experimental Package Available

  • The fix candidate is published as thin-hook@0.4.0-alpha.49-experimental.2 with experimental tag

Design Issues

  • Should @thin-hook/dependencies be published independently to put it under "dependencies" instead of "optionalDependencies"?

Fix Candidate

  • Load thin-hook dependencies via @thin-hook/dependencies/index.js

plugins/dependencies/package.json

{
  "name": "@thin-hook/dependencies",
  "version": "0.4.0-alpha.49-experimental.2",
  "description": "Thin Hook Preprocessor dependencies",
  "main": "index.js",
  "scripts": {
  },
  "bin": {
  },
  "repository": {
    "type": "git",
    "url": "git+https://github.com/t2ym/thin-hook.git"
  },
  "keywords": [
    "ES6",
    "ES2015",
    "hook"
  ],
  "author": "Tetsuya Mori <t2y3141592@gmail.com>",
  "license": "BSD-2-Clause",
  "bugs": {
    "url": "https://github.com/t2ym/thin-hook/issues"
  },
  "homepage": "https://github.com/t2ym/thin-hook#readme",
  "dependencies": {
    "convert-source-map": "^1.5.0",
    "escodegen": "github:t2ym/escodegen#master",
    "espree": "github:t2ym/espree#master",
    "he": "^1.1.1",
    "htmlparser2": "3.9.2",
    "import-maps": "github:t2ym/import-maps#browserify",
    "pako": "^1.0.10",
    "sha.js": "^2.4.8"
  },
  "optionalDependencies": {
  },
  "devDependencies": {
  }
}

plugins/dependencies/index.js

/*
@license https://github.com/t2ym/thin-hook/blob/master/LICENSE.md
Copyright (c) 2020, Tetsuya Mori <t2y3141592@gmail.com>. All rights reserved.
*/
module.exports = {
  espree: require('espree'),
  escodegen: require('escodegen'),
  htmlparser: require('htmlparser2'),
  createHash: require('sha.js'),
  convert: require('convert-source-map'),
  he: require('he'),
  zlib: require('pako'),
  parseFromString: require('import-maps/reference-implementation/lib/parser.js').parseFromString,
  resolve: require('import-maps/reference-implementation/lib/resolver.js').resolve,
};
diff --git a/hook.js b/hook.js
index bfef7da7..c53e83ba 100644
--- a/hook.js
+++ b/hook.js
@@ -1,17 +1,9 @@
 /*
 @license https://github.com/t2ym/thin-hook/blob/master/LICENSE.md
-Copyright (c) 2017, 2018, Tetsuya Mori <t2y3141592@gmail.com>. All rights reserved.
+Copyright (c) 2017, 2018, 2019, 2020 Tetsuya Mori <t2y3141592@gmail.com>. All rights reserved.
 */
 
-const espree = require('espree');
-const escodegen = require('escodegen');
-const htmlparser = require('htmlparser2');
-const createHash = require('sha.js');
-const convert = require('convert-source-map');
-const he = require('he');
-const zlib = require('pako');
-const { parseFromString } = require('import-maps/reference-implementation/lib/parser.js');
-const { resolve } = require('import-maps/reference-implementation/lib/resolver.js');
+const { espree, escodegen, htmlparser, createHash, convert, he, zlib, parseFromString, resolve } = require('@thin-hook/dependencies');
 const preprocess = require('./lib/preprocess.js')(espree, escodegen, htmlparser, createHash, convert, he);
 const hook = preprocess.hook;
 const serviceWorker = require('./lib/service-worker.js')(hook, preprocess);
diff --git a/package.json b/package.json
index f8a44b89..75395d1f 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "thin-hook",
-  "version": "0.4.0-alpha.48",
+  "version": "0.4.0-alpha.49-experimental.2",
   "description": "Thin Hook Preprocessor",
   "main": "hook.js",
   "scripts": {
@@ -50,16 +50,7 @@
     "url": "https://github.com/t2ym/thin-hook/issues"
   },
   "homepage": "https://github.com/t2ym/thin-hook#readme",
-  "dependencies": {
-    "convert-source-map": "^1.5.0",
-    "escodegen": "github:t2ym/escodegen#master",
-    "espree": "github:t2ym/espree#master",
-    "he": "^1.1.1",
-    "htmlparser2": "3.9.2",
-    "import-maps": "github:t2ym/import-maps#browserify",
-    "pako": "^1.0.10",
-    "sha.js": "^2.4.8"
-  },
+  "dependencies": {},
   "optionalDependencies": {
     "@thin-hook/about-blank-redirector": "file:plugins/about-blank-redirector",
     "@thin-hook/automation-secret": "file:plugins/automation-secret",
@@ -75,6 +66,7 @@
     "@thin-hook/clean-gzip-json": "file:plugins/clean-gzip-json",
     "@thin-hook/content-loader-js": "file:plugins/content-loader-js",
     "@thin-hook/context-generator-js": "file:plugins/context-generator-js",
+    "@thin-hook/dependencies": "file:plugins/dependencies",
     "@thin-hook/disable-devtools": "file:plugins/disable-devtools",
     "@thin-hook/entry-page": "file:plugins/entry-page",
     "@thin-hook/frontend-components": "file:plugins/frontend-components",
diff --git a/plugins/injector-helpers/configurator.js b/plugins/injector-helpers/configurator.js
index c0fa1dee..00b44a66 100644
--- a/plugins/injector-helpers/configurator.js
+++ b/plugins/injector-helpers/configurator.js
@@ -8,16 +8,7 @@ const { HtmlInjectionHandlerFactory } = require('target-injector/HtmlInjectionHa
 const { JsInjectionHandlerFactory } = require('target-injector/JsInjectionHandlerFactory.js');
 
 const { Parser } = require("htmlparser2");
-const { DomHandler } = require("domhandler").DomHandler
-  ? require("domhandler") // domhandler@3
-  : require(require.resolve("domhandler", { // avoid loading domhandler@2, which is a dependency of htmlparser2@3.9.2
-      paths: require.resolve.paths("domhandler")
-        .filter(_path =>
-          !_path.endsWith('/thin-hook/plugins/injector-helpers/node_modules') &&
-          !_path.endsWith('/thin-hook/plugins/node_modules') &&
-          !_path.endsWith('/thin-hook/node_modules')
-        )
-    }));
+const { DomHandler } = require("domhandler");
 const cssauron = require('cssauron');
 
 const parser = require('espree'); // can be esprima or acorn
@t2ym t2ym changed the title [plugin] Plugin dependencies can be overridden by thin-hook dependencies (generalized #402) [plugin] Plugin dependencies can be unexpectedly overridden by thin-hook dependencies (generalized #402) Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant