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

Separate third party inline scripts #241

Merged

Conversation

PaperStrike
Copy link
Member

@PaperStrike PaperStrike commented Apr 5, 2021

PR Checklist

  • The commit message follows guidelines for NexT.
  • Tests for the changes was made (for bug fixes / features).
    • analytics
      • baidu-analytics
      • google-analytics
      • growingio
    • chat
      • chatra
    • comments
      • changyan
      • disqus
      • disqusjs
      • gitalk
      • isso
      • livere
      • utterances
    • math
      • katex
      • mathjax
    • statistics
      • firestore
      • lean-analytics
    • tags
      • mermaid
      • pdf
    • nprogress
    • quicklink
    • rating

The main problem is there are so many third party scrips to test.

PR Type

  • Refactoring (no functional changes, no api changes).

What is the current behavior?

Continue working on #226! Another pull request for more dedicated tests!

Issue resolved: #220

What is the new behavior?

Links below, if not specific, redirect to the related code line(s) / file.

IMPORTANT & Requires Discussions:

@github-actions github-actions bot added the Layout label Apr 5, 2021
@PaperStrike PaperStrike force-pushed the no-third-party-inline-script branch from 9d361f6 to 211dcdb Compare April 5, 2021 14:46
@PaperStrike PaperStrike added this to the 8.4.0 milestone Apr 5, 2021
@PaperStrike
Copy link
Member Author

Google Analytics and WidgetPack Rating on Pjax sites seem to have been broken ever since Pjax is introduced, long before this PR.

An unexpected thing is that Widget Pack's official script uses Function constructor, passes script code in URL(s), and injects inline stylesheets, which leads to CSP rules: unsafe-eval and unsafe-inline in script-src, and unsafe-inline in style-src.
I can't solve this.

- Not listen to Pjax related events since GrowingIO listens to URL change events on its own
- Resolve a conflict with copy_button
- Replace `getScript` by original `getScriptPromise`, `loadComments` by original `loadCommentsPromise`
- Expose less properties of CONFIG
@PaperStrike PaperStrike marked this pull request as ready for review April 8, 2021 13:29
- Deprecate quicklink.ignores but not obsolete it
- Rename `load-config.js` in source/js/ to `config.js`
@njzjz
Copy link
Member

njzjz commented Apr 9, 2021

Or add third party scripts to route only when needed? If the latter is better, how should we achieve it?

I have an idea:

hexo.extend.filter.register('after_generate', () => {
  // minify js files
  hexo.route.list().filter(path => (all_js.includes(path) && !used_js.includes(path))).forEach(path => {
    hexo.route.remove(path);
  });
});

used_js can be added in next_js.

@PaperStrike
Copy link
Member Author

PaperStrike commented Apr 9, 2021

used_js can be added in next_js.

I tried before but failed. It seems like next_js runs at the render stage, after the generate stage, which means used_js will not be ready at all at that time. And hexo.route.add mysteriously not functioning when called in next_js. 🤔

I might be wrong anyway.

@PaperStrike
Copy link
Member Author

Status Update: Function and Regex in config file's quicklink.ignores will be deprecated once merged.

Theme config file has been updated:

hexo-theme-next/_config.yml

Lines 554 to 557 in 7a5b348

# For more flexibility you can add some patterns (RegExp, Function, or Array) to ignores.
# See: https://github.com/GoogleChromeLabs/quicklink#custom-ignore-patterns
# This option is deprecated. Use `CONFIG.quicklink.ignores` in your custom scripts instead.
ignores:

If used, error message will be shown as:

// eslint-disable-next-line no-console
console.error('Setting regex and function in config file is deprecated. Try setting `CONFIG.quicklink.ignores` in any script code.');

Users using this config should turn to configure CONFIG.quicklink.ignores in any custom script code / file.

@PaperStrike
Copy link
Member Author

PaperStrike commented Apr 10, 2021

The rest comment systems are hard for me to test, I suggest merging now and getting some feedback.

@PaperStrike PaperStrike merged commit a66b930 into next-theme:master Apr 10, 2021
@PaperStrike PaperStrike deleted the no-third-party-inline-script branch April 10, 2021 17:18
PaperStrike added a commit to PaperStrike/hexo-theme-next that referenced this pull request Apr 29, 2021
PaperStrike added a commit to PaperStrike/hexo-theme-next that referenced this pull request Apr 29, 2021
PaperStrike added a commit to PaperStrike/hexo-theme-next that referenced this pull request Apr 29, 2021
@PaperStrike
Copy link
Member Author

PaperStrike commented Apr 29, 2021

To NexT plugin authors,

Sorry that I forgot the plugins which are outside this repo and use NexT.utils.getScript, NexT.utils.loadComments, or bodyEnd inject point. I merged this after only a simple search.

As you may have seen in the PR description,

  • NexT.utils.getScript and NexT.utils.loadComments are refactored.
  • bodyEnd will not be refreshed on Pjax reloads anymore.

NexT.utils.getScript and NexT.utils.loadComments now returns a Promise, so your callback functions can be associated by using its then method. Like:

// Before:
NexT.utils.getScript('example.js', onLoad);

// After:
NexT.utils.getScript('example.js').then(onLoad);
NexT.utils.getScript('example.js').then(onLoad, onError); // better

The condition parameter of NexT.utils.getScript is repositioned and the function now accepts more options. If you need to have your <script> more customized, try it now:

// Before:
NexT.utils.getScript('example.js', onLoad, window.example);

// After:
NexT.utils.getScript('example.js', { condition: window.example }).then(onLoad, onError);

// Before:
const script = document.createElement('script');
script.async = true;
script.dataset.foo = 'bar';
script.onload = onLoad;
script.url = 'example.js';
document.getElementById('example').appendChild(script);

// After:
NexT.utils.getScript('example.js', {
  attributes: {
    async: true,
    dataset: { foo: 'bar' }
  },
  parentNode: document.getElementById('example')
}).then(onLoad, onError);

To have more precise and elegant control, bodyEnd, along with scripts inside, will not be reloaded by Pjax now. To keep your plugin Pjax-compatible,

  • make sure the elements in your plugin are consistent in every page,

and when needed,

  • try using the data-pjax attribute to mark a whole <script> to be reloaded automatically
  • or listen to pjax:success (fired when a Pjax request succeeds) or the new page:loaded (fired when DOMContentLoaded or a Pjax request succeeds) events to reload some functions. (always preferred)
{# Before: #}
{%- if theme.example.enable %}
  <script src="example.js"></script>
  {%- if is_home() %}
    <link rel="stylesheet" href="example.css">
    <script src="example.home.js"></script>
    <script>
      window.example.home.load({{ theme.example.id }});
    </script>
  {%- endif %}
{%- endif %}


{# After: #}
{%- if theme.example.enable %}
  <link rel="stylesheet" href="example.css">
  <script{{ pjax }} src="example.js"></script>
  {{ next_data('example', theme.example) }}
  <script>
    document.addEventListener('page:loaded', () => {
      if (!CONFIG.page.isHome) return;
      NexT.utils.getScript('example.home.js', { condition: window.example.home })
        .then(() => {
          window.example.home.load(CONFIG.example.id);
        });
    });
  </script>
{%- endif %}

The new next_data helper is used to export data to front-end scripts. The first parameter is the name of the exported data, the data will then be accessible as CONFIG[name] in the front-end. The rest parameter(s), if more than one, will be combined into one single Object, and then be exported as the data.

Besides, as the initial focus of this PR, it's recommended to seperate the inline scripts into single file(s).

If you want to keep supporting older versions of NexT, see #259 (comment), or use your previous code directly when next_data is not defined.

{%- if next_data %}
  {# For NexT>=8.4.0 #}
{%- else %}
  {# For NexT<=8.3.0 #}
{%- endif %}

Kind regards,
PaperStrike

PaperStrike added a commit that referenced this pull request Apr 30, 2021
stevenjoezhang referenced this pull request in next-theme/hexo-next-exif Aug 28, 2021
lingyf pushed a commit to lingyf/hexo-theme-next that referenced this pull request Jan 27, 2022
lingyf pushed a commit to lingyf/hexo-theme-next that referenced this pull request Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

移除对 CSP 响应头 'unsafe-inline' 的依赖
3 participants