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

Current browser assets not strict-mode safe (langApiRestored) #2234

Closed
masx200 opened this issue Oct 29, 2019 · 21 comments · Fixed by #2312
Closed

Current browser assets not strict-mode safe (langApiRestored) #2234

masx200 opened this issue Oct 29, 2019 · 21 comments · Fixed by #2312
Labels
help welcome Could use help from community package/build Issues relating to npm or packaging

Comments

@masx200
Copy link

masx200 commented Oct 29, 2019

In strict mode, running 'highlight.min.js' will throw an error.
Probably because the official compression destroys the properties of some objects
The file version obtained from the npm repository is different from the file obtained from other cdn repositories

import('https://cdn.staticfile.org/highlight.js/9.15.10/highlight.min.js').catch(console.error)
TypeError: Cannot create property 'langApiRestored' on string 'self'
    at i (highlight.min.js:2)
    at Array.forEach (<anonymous>)
    at i (highlight.min.js:2)
    at Array.forEach (<anonymous>)
    at i (highlight.min.js:2)
    at Array.forEach (<anonymous>)
    at i (highlight.min.js:2)
    at Array.forEach (<anonymous>)
    at i (highlight.min.js:2)
    at Object.n.registerLanguage (highlight.min.js:2)
    function i(e) {
        if (a && !e.langApiRestored) {
            for (var t in e.langApiRestored = !0,
            a)
                e[t] && (e[a[t]] = e[t]);
            (e.c || []).concat(e.v || []).forEach(i)
        }
    }
@joshgoebel
Copy link
Member

joshgoebel commented Oct 29, 2019

I'm not sure the exported CDN code is guaranteed to be strict safe. Is this because we're using a key with .access instead of using it with a string key? Do you know?

What environment is this is?

@masx200
Copy link
Author

masx200 commented Oct 29, 2019

It may be that when the official build of the compressed version, there is a problem with the compression, causing the code to run incorrectly.

When I use the following three versions of the file provided by cdn, the operation will not report an error, they are all uncompressed versions.

The following versions are the same as the npm version

It can be found that the official compression destroys the properties of some objects.

obj.contains=>e.c

obj.variants=>e.v

https://cdn.jsdelivr.net/npm/highlight.js@9.15.10/lib/highlight.js

https://unpkg.com/highlight.js@9.15.10/lib/highlight.js

https://cdn.jsdelivr.net/npm/highlight.js@9.15.10/lib/highlight.min.js

 function restoreLanguageApi(obj) {
    if(API_REPLACES && !obj.langApiRestored) {
      obj.langApiRestored = true;
      for(var key in API_REPLACES)
        obj[key] && (obj[API_REPLACES[key]] = obj[key]);
      (obj.contains || []).concat(obj.variants || []).forEach(restoreLanguageApi);
    }
  }
 function m(e) {
        if (n && !e.langApiRestored) {
            for (var t in e.langApiRestored = !0,
            n)
                e[t] && (e[n[t]] = e[t]);
            (e.contains || []).concat(e.variants || []).forEach(m)
        }
    }

@masx200 masx200 changed the title In strict mode, running 'highlight.js' will throw an error. In strict mode, running 'highlight.min.js' will throw an error.Probably because the official compression destroys the properties of some objects. Oct 29, 2019
@joshgoebel
Copy link
Member

Well, it compresses them and then restores them, that's the whole idea... sounds like strict mode just doesn't like that?... That type of compression is likely going away in the next release in any case once we have a new build system in place.

If you're using node though you might want to look at using the NPM library version and requiring it, vs trying to import the CDN version.

@masx200
Copy link
Author

masx200 commented Oct 29, 2019

Well, it compresses them and then restores them, that's the whole idea... sounds like strict mode just doesn't like that?... That type of compression is likely going away in the next release in any case once we have a new build system in place.

If you're using node though you might want to look at using the NPM library version and requiring it, vs trying to import the CDN version.

The version I downloaded from the official website also throws an exception at runtime.

This file version is the same as other cdn file versions.

https://highlightjs.org/download/

/*! highlight.js v9.15.10 | BSD3 License | git.io/hljslicense */

import('http://127.0.0.1:8080/highlight.pack.js')

When using a dynamically loaded module, the module will automatically run in strict mode

This is equivalent to automatically adding 'use strict' in front of the code.

highlight.pack.js:formatted:113 Uncaught (in promise) TypeError: Cannot create property 'langApiRestored' on string 'self'
    at i (highlight.pack.js:formatted:113)
    at Array.forEach (<anonymous>)
    at i (highlight.pack.js:formatted:116)
    at Array.forEach (<anonymous>)
    at i (highlight.pack.js:formatted:116)
    at Array.forEach (<anonymous>)
    at i (highlight.pack.js:formatted:116)
    at Object.a.registerLanguage (highlight.pack.js:formatted:440)
    at highlight.pack.js:formatted:595
function i(e) {
        if (r && !e.langApiRestored) {
            for (var n in e.langApiRestored = !0,
            r)
                e[n] && (e[r[n]] = e[n]);
            (e.c || []).concat(e.v || []).forEach(i)
        }
    }

@joshgoebel
Copy link
Member

Again, what environment is this you are using Highlight.js in? Browser (which versions, etc), Node.js, other?

@masx200
Copy link
Author

masx200 commented Oct 29, 2019

Again, what environment is this you are using Highlight.js in? Browser (which versions, etc), Node.js, other?

the environment is browser latest chrome,

@joshgoebel
Copy link
Member

Yeah, the code/deliverables we currently ship are not ready to be used in a ES6 module context in the browser.

What does import even do in this context since our source code doesn't export anything currently...?

@masx200
Copy link
Author

masx200 commented Oct 29, 2019

However, in strict mode, running an uncompressed file version of npm will not throw an exception!
I am just testing if it works in strict mode, because strict mode may find some potential errors.

@masx200
Copy link
Author

masx200 commented Oct 29, 2019

If you use rollup to package files that depend on highlight.js, rollup forces a strict mode for each module.
Rollup will convert the umd module into an esm module!

@joshgoebel
Copy link
Member

However, in strict mode, running an uncompressed file version of npm will not throw an exception!

That's good new because it means when we stop doing the weird compression stuff that we might just be strict-mode safe. :-)

@joshgoebel
Copy link
Member

If you use rollup to package files that depend on highlight.js, rollup forces a strict mode for each module. Rollup will convert the umd module into an esm module!

I'm wanting to switch to ESM modules and build our deliverables with rollup in any case, but I hadn't decided we need to turn on strict mode for sure or not...

@joshgoebel joshgoebel changed the title In strict mode, running 'highlight.min.js' will throw an error.Probably because the official compression destroys the properties of some objects. Current browser assets not strict-mode safe. Oct 29, 2019
@joshgoebel joshgoebel added cdn package/build Issues relating to npm or packaging labels Oct 29, 2019
@masx200
Copy link
Author

masx200 commented Oct 29, 2019

It is recommended to use the terser compression tool to compress the code.

Ensure that the object properties in the code are not destroyed.

@joshgoebel
Copy link
Member

Terser was what I was looking at IIRC.

@masx200
Copy link
Author

masx200 commented Oct 29, 2019

After switching to the es module, there is also the advantage that the code can be "tree shaking".

@joshgoebel
Copy link
Member

joshgoebel commented Oct 29, 2019

All the code is pretty much required and we don't really have any dependencies so tree-shaking isn't a big win for us I don't think.

@joshgoebel
Copy link
Member

Actually this might be fixable with:

  function restoreLanguageApi(obj) {
    if (typeof obj === "string") return;

I think the whole issue is that "self" is passed sometimes and it's choking up on that.

@joshgoebel
Copy link
Member

If anyone wants to confirm or make a PR I think we'd accept that if it helps anyone with strict issues ATM.

@joshgoebel
Copy link
Member

@masx200 Would that actually fix your whole issue here?

@masx200
Copy link
Author

masx200 commented Nov 12, 2019

@masx200 Would that actually fix your whole issue here?

awesome

@joshgoebel
Copy link
Member

It was a question.

@joshgoebel joshgoebel added the help welcome Could use help from community label Nov 21, 2019
@joshgoebel joshgoebel changed the title Current browser assets not strict-mode safe. Current browser assets not strict-mode safe (langApiRestored) Dec 5, 2019
@joshgoebel joshgoebel added this to the 10.0 milestone Dec 26, 2019
@joshgoebel
Copy link
Member

Closing this as no activity and the new build system (coming soon) should also resolve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help welcome Could use help from community package/build Issues relating to npm or packaging
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants