ProseMirror / prosemirror

The ProseMirror WYSIWYM editor
http://prosemirror.net/
MIT License
7.59k stars 335 forks source link

Large diff when running pm-buildhelper #1410

Closed bradleyayers closed 1 year ago

bradleyayers commented 1 year ago

I'm in the process of upgrading to the latest version of prosemirror-* packages, and need to migrate some of our patches across. However I'm finding that when running the prepare NPM script the output dist is quite different from the code in the published NPM packages.

e.g. from prosemirror-view

diff --git a/dist/index.cjs b/dist/index.cjs
index adcf38c144f43bed6d81b0c0a00bf254019ab957..ce5cd95c74abc469d369bdfe8569c120a2dfe9ed 100644
--- a/dist/index.cjs
+++ b/dist/index.cjs
@@ -1,78 +1,53 @@
 'use strict';

-function _typeof(obj) { "@babel/helpers - typeof"; return _typeof = "function" == typeof Symbol && "symbol" == typeof Symbol.iterator ? function (obj) { return typeof obj; } : function (obj) { return obj && "function" == typeof Symbol && obj.constructor === Symbol && obj !== Symbol.prototype ? "symbol" : typeof obj; }, _typeof(obj); }
-
 function _createForOfIteratorHelper(o, allowArrayLike) { var it = typeof Symbol !== "undefined" && o[Symbol.iterator] || o["@@iterator"]; if (!it) { if (Array.isArray(o) || (it = _unsupportedIterableToArray(o)) || allowArrayLike && o && typeof o.length === "number") { if (it) o = it; var i = 0; var F = function F() {}; return { s: F, n: function n() { if (i >= o.length) return { done: true }; return { done: false, value: o[i++] }; }, e: function e(_e) { throw _e; }, f: F }; } throw new TypeError("Invalid attempt to iterate non-iterable instance.\nIn order to be iterable, non-array objects must have a [Symbol.iterator]() method."); } var normalCompletion = true, didErr = false, err; return { s: function s() { it = it.call(o); }, n: function n() { var step = it.next(); normalCompletion = step.done; return step; }, e: function e(_e2) { didErr = true; err = _e2; }, f: function f() { try { if (!normalCompletion && it["return"] != null) it["return"](); } finally { if (didErr) throw err; } } }; }
-
 function _unsupportedIterableToArray(o, minLen) { if (!o) return; if (typeof o === "string") return _arrayLikeToArray(o, minLen); var n = Object.prototype.toString.call(o).slice(8, -1); if (n === "Object" && o.constructor) n = o.constructor.name; if (n === "Map" || n === "Set") return Array.from(o); if (n === "Arguments" || /^(?:Ui|I)nt(?:8|16|32)(?:Clamped)?Array$/.test(n)) return _arrayLikeToArray(o, minLen); }
-
-function _arrayLikeToArray(arr, len) { if (len == null || len > arr.length) len = arr.length; for (var i = 0, arr2 = new Array(len); i < len; i++) { arr2[i] = arr[i]; } return arr2; }
-
-function _get() { if (typeof Reflect !== "undefined" && Reflect.get) { _get = Reflect.get; } else { _get = function _get(target, property, receiver) { var base = _superPropBase(target, property); if (!base) return; var desc = Object.getOwnPropertyDescriptor(base, property); if (desc.get) { return desc.get.call(arguments.length < 3 ? target : receiver); } return desc.value; }; } return _get.apply(this, arguments); }
-
+function _arrayLikeToArray(arr, len) { if (len == null || len > arr.length) len = arr.length; for (var i = 0, arr2 = new Array(len); i < len; i++) arr2[i] = arr[i]; return arr2; }
+function _get() { if (typeof Reflect !== "undefined" && Reflect.get) { _get = Reflect.get.bind(); } else { _get = function _get(target, property, receiver) { var base = _superPropBase(target, property); if (!base) return; var desc = Object.getOwnPropertyDescriptor(base, property); if (desc.get) { return desc.get.call(arguments.length < 3 ? target : receiver); } return desc.value; }; } return _get.apply(this, arguments); }
 function _superPropBase(object, property) { while (!Object.prototype.hasOwnProperty.call(object, property)) { object = _getPrototypeOf(object); if (object === null) break; } return object; }
-
 function _inherits(subClass, superClass) { if (typeof superClass !== "function" && superClass !== null) { throw new TypeError("Super expression must either be null or a function"); } subClass.prototype = Object.create(superClass && superClass.prototype, { constructor: { value: subClass, writable: true, configurable: true } }); Object.defineProperty(subClass, "prototype", { writable: false }); if (superClass) _setPrototypeOf(subClass, superClass); }
-
-function _setPrototypeOf(o, p) { _setPrototypeOf = Object.setPrototypeOf || function _setPrototypeOf(o, p) { o.__proto__ = p; return o; }; return _setPrototypeOf(o, p); }
-
+function _setPrototypeOf(o, p) { _setPrototypeOf = Object.setPrototypeOf ? Object.setPrototypeOf.bind() : function _setPrototypeOf(o, p) { o.__proto__ = p; return o; }; return _setPrototypeOf(o, p); }
 function _createSuper(Derived) { var hasNativeReflectConstruct = _isNativeReflectConstruct(); return function _createSuperInternal() { var Super = _getPrototypeOf(Derived), result; if (hasNativeReflectConstruct) { var NewTarget = _getPrototypeOf(this).constructor; result = Reflect.construct(Super, arguments, NewTarget); } else { result = Super.apply(this, arguments); } return _possibleConstructorReturn(this, result); }; }
-
 function _possibleConstructorReturn(self, call) { if (call && (_typeof(call) === "object" || typeof call === "function")) { return call; } else if (call !== void 0) { throw new TypeError("Derived constructors may only return object or undefined"); } return _assertThisInitialized(self); }
-
 function _assertThisInitialized(self) { if (self === void 0) { throw new ReferenceError("this hasn't been initialised - super() hasn't been called"); } return self; }
-
 function _isNativeReflectConstruct() { if (typeof Reflect === "undefined" || !Reflect.construct) return false; if (Reflect.construct.sham) return false; if (typeof Proxy === "function") return true; try { Boolean.prototype.valueOf.call(Reflect.construct(Boolean, [], function () {})); return true; } catch (e) { return false; } }
-
-function _getPrototypeOf(o) { _getPrototypeOf = Object.setPrototypeOf ? Object.getPrototypeOf : function _getPrototypeOf(o) { return o.__proto__ || Object.getPrototypeOf(o); }; return _getPrototypeOf(o); }
-
+function _getPrototypeOf(o) { _getPrototypeOf = Object.setPrototypeOf ? Object.getPrototypeOf.bind() : function _getPrototypeOf(o) { return o.__proto__ || Object.getPrototypeOf(o); }; return _getPrototypeOf(o); }
+function _typeof(obj) { "@babel/helpers - typeof"; return _typeof = "function" == typeof Symbol && "symbol" == typeof Symbol.iterator ? function (obj) { return typeof obj; } : function (obj) { return obj && "function" == typeof Symbol && obj.constructor === Symbol && obj !== Symbol.prototype ? "symbol" : typeof obj; }, _typeof(obj); }
 function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }
-
-function _defineProperties(target, props) { for (var i = 0; i < props.length; i++) { var descriptor = props[i]; descriptor.enumerable = descriptor.enumerable || false; descriptor.configurable = true; if ("value" in descriptor) descriptor.writable = true; Object.defineProperty(target, descriptor.key, descriptor); } }
-
+function _defineProperties(target, props) { for (var i = 0; i < props.length; i++) { var descriptor = props[i]; descriptor.enumerable = descriptor.enumerable || false; descriptor.configurable = true; if ("value" in descriptor) descriptor.writable = true; Object.defineProperty(target, _toPropertyKey(descriptor.key), descriptor); } }
 function _createClass(Constructor, protoProps, staticProps) { if (protoProps) _defineProperties(Constructor.prototype, protoProps); if (staticProps) _defineProperties(Constructor, staticProps); Object.defineProperty(Constructor, "prototype", { writable: false }); return Constructor; }
-
+function _toPropertyKey(arg) { var key = _toPrimitive(arg, "string"); return _typeof(key) === "symbol" ? key : String(key); }
+function _toPrimitive(input, hint) { if (_typeof(input) !== "object" || input === null) return input; var prim = input[Symbol.toPrimitive]; if (prim !== undefined) { var res = prim.call(input, hint || "default"); if (_typeof(res) !== "object") return res; throw new TypeError("@@toPrimitive must return a primitive value."); } return (hint === "string" ? String : Number)(input); }
 Object.defineProperty(exports, '__esModule', {
   value: true
 });

I'm guessing there are transitive dependencies that have upgraded in the background. Perhaps including a package-lock.json in the repo would help in this scenario.

marijnh commented 1 year ago

Are you applying patches to the build output, or why is this a problem for you?

The ESM output doesn't use Babel, by the way, and might be more stable.

bradleyayers commented 1 year ago

Yes we have patches that apply to the build output, as we a number of runtime behavior changes in our patches.

Unfortunately we need CJS support, so we have to patch those.

I've tried a couple of workflows:

  1. yarn patch prosemirror-view
  2. Make changes to src/…
  3. Build outputs via npm install
  4. Save patch via yarn patch-commit -s …

This is pretty simple but doens't produce min-diff outputs.

I then just tried cloning the repo and building from that, and had similar results.

At first the problem I was having was just "verbose diff", but for some other packages (e.g. prosemirror-markdown) I actually get broken output, e.g.

 function tokenHandlers(schema, tokens) {
   var handlers = Object.create(null);
-
-  var _loop = function _loop(type) {
+  var _loop = function _loop() {
     var spec = tokens[type];
-
     if (spec.block) {
       var nodeType = schema.nodeType(spec.block);
-
       if (noCloseToken(spec, type)) {
         handlers[type] = function (state, tok, tokens, i) {
           state.openNode(nodeType, attrs(spec, tok, tokens, i));
@@ -438,20 +412,17 @@ function tokenHandlers(schema, tokens) {
         handlers[type + "_open"] = function (state, tok, tokens, i) {
           return state.openNode(nodeType, attrs(spec, tok, tokens, i));
         };
-
         handlers[type + "_close"] = function (state) {
           return state.closeNode();
         };
       }
     } else if (spec.node) {
       var _nodeType = schema.nodeType(spec.node);
-
       handlers[type] = function (state, tok, tokens, i) {
         return state.addNode(_nodeType, attrs(spec, tok, tokens, i));
       };
     } else if (spec.mark) {
       var markType = schema.marks[spec.mark];
-
       if (noCloseToken(spec, type)) {
         handlers[type] = function (state, tok, tokens, i) {
           state.openMark(markType.create(attrs(spec, tok, tokens, i)));
@@ -462,7 +433,6 @@ function tokenHandlers(schema, tokens) {
         handlers[type + "_open"] = function (state, tok, tokens, i) {
           return state.openMark(markType.create(attrs(spec, tok, tokens, i)));
         };
-
         handlers[type + "_close"] = function (state) {
           return state.closeMark(markType);
         };
@@ -478,63 +448,47 @@ function tokenHandlers(schema, tokens) {
       throw new RangeError("Unrecognized parsing spec " + JSON.stringify(spec));
     }
   };
-
   for (var type in tokens) {
-    _loop(type);
+    _loop();
   }

This looks really broken, like type is used…

So at this point I can either manually patch the build outputs, or try to figure out what version of dependencies will produce the right outputs.

marijnh commented 1 year ago

Patching build output is a rather unconventional thing to do, and not something this library provides any guarantees for, sorry. Seems like applying the patches to the input sources and running the build process on the result would work better.

bradleyayers commented 1 year ago

Ah I think I might have misunderstood the question. Yes I'm patching input sources (i.e. Make changes to src/…), running the build process (npm install which in turn runs npm run prepare which in turn runs pm-buildhelper), and then I'm seeing large diffs in the build output.

I'm then saving a patch of the build output (this is how yarn patch and patch-package work). Unless I'm misunderstanding something if you git clone prosemirror-markdown and run npm install, you'll see corrupted outputs in dist/ too.

marijnh commented 1 year ago

I see. But this really is a problem that I consider out of scope from what the library guarantees.

bradleyayers commented 1 year ago

I guess I found it surprising that cloning prosemirror-markdown and running npm install -> npm prepare produces broken dist/, but if it's working for you fair enough.

Thanks for the consideration and quick replies.