spatialillusions / milsymbol

Military Symbols in JavaScript
www.spatialillusions.com/milsymbol
MIT License
544 stars 133 forks source link

Problem using `dist/milsymbol.esm.js` from milsymbol NPM package #245

Closed claws closed 1 year ago

claws commented 4 years ago

I'm trying to use the bundled ESM version of milsymbol (dist/milsymbol.esm.js) provided in the NPM package but I encounter an error when trying to create a milsymbol using asCanvas.

The error I see is:

milsymbol.esm.js:3101 Uncaught TypeError: Cannot add property _brokenPath2D, object is not extensible
    at Symbol.asCanvas (milsymbol.esm.js:3101)
    at index.html:79

I have tried Chrome and Safari browser. I could not find any existing open issues that were similar.

After a little investigation I found that at the end of the dist/milsymbol.esm.js file there is a line let milsymbol = Object.freeze(ms);. Freeze prevents any modification to the ms object. So, later when the demo tries to run the asCanvas function there is a line that confirms Path2D is working. The line ms._brokenPath2D = !(data == "0,0,0,255"); fails because _brokenPath2D is not an existing property of ms and could not be modified even if it were.

I'm relatively new to JavaScript development so perhaps the way I'm trying to use this package is not supported. I'd like to use the prebuilt bundled version if possible. I'm trying to use ES6 style imports.

Should I be able to use the dist/milsymbol.esm.js file provided in the milsymbol npm package?

Below is the simplest way I can show you how to reproduce the issue I observe.

$ mkdir test-milsysmbol
$ cd test-milsymbol
$ npm init
$ npm install milsymbol

Fetch the milsymbol es6 example to use as a local demo.

$ wget https://raw.githubusercontent.com/spatialillusions/milsymbol/master/examples/es6-import/index.html

Modify index.html file to use the bundled ESM file dist/milsymbol.esm.js that comes in the milsymbol npm package.

Change index.html line 55 from:

import { ms, std2525c } from '../../index.esm.js';

to:

import { milsymbol as ms, std2525c } from './node_modules/milsymbol/dist/milsymbol.esm.js';

Install and run a simple web server to serve the index.html file.

$ npm install http-server
$ ./node_modules/http-server/bin/http-server -a localhost

View results in your browser.

I expect to see 4 icons (two properly displayed and two with an upside down question mark). I see only one icon (the first SVG icon) and the browser Dev Tools shows the following error:

milsymbol.esm.js:3101 Uncaught TypeError: Cannot add property _brokenPath2D, object is not extensible
    at Symbol.asCanvas (milsymbol.esm.js:3101)
    at index.html:79
spatialillusions commented 4 years ago

This is sort of strange, because you should not need the brokenPath2D when you have esm support, the brokenPath2D is a polyfill for old versions of Internet Explorer that cant draw SVG paths to the canvas, so it is strange to see that you get this behavior.

Have you tried having a look at the included esm sample? https://github.com/spatialillusions/milsymbol/blob/master/examples/es6-import/index.html Live version https://spatialillusions.com/milsymbol/examples/es6-import/

I can see that it isn't working as it should when you haven't loaded the necessary functions, but otherwise it works fine.

claws commented 4 years ago

I did look at the esm example you pointed out. I used that same file in the sequence of steps I attached that show how to reproduce the problem I observe.

The live version you point out uses the package source code (index.esm.js) not the bundled dist/milsymbol.esm.js which is why it does not exhibit the problem. If you swap the live example to use the bundled version (dist/milsymbol.esm.js) I think you would see exactly what I see.

I agree that the _brokenPath2D should not be used when Path2D exists. However, the code in the asCanvas function tries to write to the _brokenPath2D property even when Path2D exists. That is the root cause of the problem when using the bundled version (dist/milsymbol.esm.js) of the package in which the ms object is frozen.

spatialillusions commented 1 year ago

Should be fixed in the version just published to NPM, otherwise let me know.