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

intl-messageformat throwing warnings #12

Closed
Sureiya opened this issue Oct 7, 2019 · 8 comments
Closed

intl-messageformat throwing warnings #12

Sureiya opened this issue Oct 7, 2019 · 8 comments

Comments

@Sureiya
Copy link

Sureiya commented Oct 7, 2019

I'm currently building a small app using the basic rollup svelte template, and svelte-i18n, and rollup is currently throwing the following warning when I build with locale or dictionary imported.

rollup v1.23.1
bundles src/main.js → public\bundle.js...
LiveReload enabled
(!) `this` has been rewritten to `undefined`
https://rollupjs.org/guide/en/#error-this-is-undefined       
node_modules\intl-messageformat\lib\core.js
4: See the accompanying LICENSE file for terms.
5: */
6: var __extends = (this && this.__extends) || (function () {
                    ^
7:     var extendStatics = function (d, b) {
8:         extendStatics = Object.setPrototypeOf ||
...and 3 other occurrences
node_modules\intl-messageformat\lib\compiler.js
4: See the accompanying LICENSE file for terms.
5: */
6: var __extends = (this && this.__extends) || (function () {
                    ^
7:     var extendStatics = function (d, b) {
8:         extendStatics = Object.setPrototypeOf ||
...and 1 other occurrence

My package.json

{
  "name": "svelte-app",
  "version": "1.0.0",
  "devDependencies": {
    "npm-run-all": "^4.1.5",
    "rollup": "^1.23.0",
    "rollup-plugin-commonjs": "^10.0.0",
    "rollup-plugin-livereload": "^1.0.0",
    "rollup-plugin-node-resolve": "^5.2.0",
    "rollup-plugin-svelte": "^5.1.0",
    "rollup-plugin-terser": "^5.1.2",
    "svelte": "^3.12.0"
  },
  "dependencies": {
    "date-fns": "^2.4.1",
    "lodash": "^4.17.15",
    "sirv-cli": "^0.4.4",
    "svelte-i18n": "^1.1.2-beta",
    "svelte-routing": "^1.4.0"
  },
  "scripts": {
    "build": "rollup -c",
    "autobuild": "rollup -c -w",
    "dev": "run-p start:dev autobuild",
    "start": "sirv public --single",
    "start:dev": "sirv public --single --dev"
  }
}

Anyone have any insights here?

@ashour
Copy link

ashour commented Oct 8, 2019

Hello @Sureiya. Yeah this seems to be a known issue with Rollup. To be consistent with native module implementations, Rollup sets this = undefined at the top level in modules. It also displays a warning if a module tries to use this at the top level. intl-messageformat has code that uses this at the top level, which upsets Rollup.

The intl-messageformat code seems to work just fine, however, and I've gotten rid of the warnings by using the suggestion in the Rollup documentation to provide the two culprit modules—node_modules\intl-messageformat\lib\core.js and node_modules\intl-messageformat\lib\compiler.js—with window instead of undefined as the this value. Since all our code runs in the browser, I'm assuming the two modules expect this = window. I've achieve the override in my rollup.config.js:

import svelte from 'rollup-plugin-svelte';
import resolve from 'rollup-plugin-node-resolve';
import commonjs from 'rollup-plugin-commonjs';
import livereload from 'rollup-plugin-livereload';
import { terser } from 'rollup-plugin-terser';

const production = !process.env.ROLLUP_WATCH;

export default {
    input: 'src/main.js',
    output: {
        sourcemap: true,
        format: 'iife',
        name: 'app',
        file: 'public/bundle.js',
    },

    // 👇🏽 The added `moduleContext` key is the relevant override here.

    moduleContext: (id) => {
        const thisAsWindowForModules = [
            'node_modules/intl-messageformat/lib/core.js',
            'node_modules/intl-messageformat/lib/compiler.js',
        ];

        if (thisAsWindowForModules.some(id_ => id.trimRight().endsWith(id_))) {
            return 'window';
        }
    },

   // 👆🏽

    plugins: [
        svelte({
            // enable run-time checks when not in production
            dev: !production,
            // we'll extract any component CSS out into
            // a separate file — better for performance
            css: css => {
                css.write('public/bundle.css');
            }
        }),

        // If you have external dependencies installed from
        // npm, you'll most likely need these plugins. In
        // some cases you'll need additional configuration —
        // consult the documentation for details:
        // https://github.com/rollup/rollup-plugin-commonjs
        resolve({
            browser: true,
            dedupe: importee => importee === 'svelte' || importee.startsWith('svelte/')
        }),
        commonjs(),

        // Watch the `public` directory and refresh the
        // browser on changes when not in production
        !production && livereload('public'),

        // If we're building for production (npm run build
        // instead of npm run dev), minify
        production && terser()
    ],
    watch: {
        clearScreen: false
    }
};

May be this should be addressed in the README doc? Happy to provide a PR if people agree.

@ashour
Copy link

ashour commented Oct 8, 2019

Oh, and there may be a related issue in the intl-messageformat repo. It seems that the var __extends = (this && this.__extends) || (function () { code is created by TypeScript, which is the language intl-messageformat is written in, I believe. Not 100% sure about this but wanted to share in case someone wants to do a bit more digging.

@kaisermann kaisermann added the help wanted Extra attention is needed label Oct 8, 2019
@kaisermann
Copy link
Owner

kaisermann commented Oct 8, 2019

As @ashour explained better than I ever could, this is a known issue related to rollup + intl-messageformat generated code. I currently don't know what could be done to prevent the warning other than changing the underlying lib. There's another one that I know of that also formats messages using the ICU syntax, but it weighs +- 4kb more 🤔

@Sureiya
Copy link
Author

Sureiya commented Oct 9, 2019

Thanks for the well documented code @ashour! I was aware of the cause and that fix, but something seems fishy here, and I think we might be overlooking something. I've went over the rollup config for formatjs and it seems fine there. The reason I reported this here is that I can't find any mention of this elsewhere, and I would have expected to see some issue on formatjs.

I've been playing all day trying to resolve this. I thought I had it fixed at one point using some different import strategies. One thing I haven't tried is giving rollup a new build target and having it fix the intl-messageformat in node_modules before building the library. Another solution would be to remove it as an external module and add the moduleContext code ashour provided. This increases the bundle size, but I believe this would end up being bundled with it anyway in the end unless I'm wrong?

@kaisermann
Copy link
Owner

kaisermann commented Nov 1, 2019

Quick update about this: i started messing around with the formatjs mono-repo and even the build script of intl-messageformat throws the same warning:
image

continues looking

For now, we have this: rollup/rollup#794 (comment). A custom onwarn handle which ignores this kind of warning.

@omirobarcelo
Copy link

Thanks for the explanations!

But event with @ashour code I have the same warning. Added, but I don't know if related, I also have the following error:

[!] Error: UMD and IIFE output formats are not supported for code-splitting builds.
Error: UMD and IIFE output formats are not supported for code-splitting builds.
    at error (C:\Users\Dev\Documents\shoguneko-dev\fanhunter-rogue\node_modules\rollup\dist\rollup.js:5365:30)
    at normalizeOutputOptions (C:\Users\Dev\Documents\shoguneko-dev\fanhunter-rogue\node_modules\rollup\dist\rollup.js:13726:13)
    at getOutputOptionsAndPluginDriver (C:\Users\Dev\Documents\shoguneko-dev\fanhunter-rogue\node_modules\rollup\dist\rollup.js:13513:32)
    at Object.write (C:\Users\Dev\Documents\shoguneko-dev\fanhunter-rogue\node_modules\rollup\dist\rollup.js:13586:63)
    at C:\Users\Dev\Documents\shoguneko-dev\fanhunter-rogue\node_modules\rollup\dist\rollup.js:17015:66
    at Array.map (<anonymous>)
    at C:\Users\Dev\Documents\shoguneko-dev\fanhunter-rogue\node_modules\rollup\dist\rollup.js:17015:45
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

I use svelte-i18n in App.svelte (I've also tried main.js) as

<script>
  import Router from "svelte-spa-router";
  import AppHeader from "./components/AppHeader.svelte";
  import routes from "./routes.js";
  import { init, register } from "svelte-i18n";

  register("en", () => import("./locales/en.json"));
  register("es", () => import("./locales/es.json"));

  init({
    fallbackLocale: "en",
    initialLocale: {
      navigator: true
    }
  });
</script>

<AppHeader />
<Router {routes} />

And my rollup.config.js is

import svelte from "rollup-plugin-svelte";
import resolve from "rollup-plugin-node-resolve";
import commonjs from "rollup-plugin-commonjs";
import livereload from "rollup-plugin-livereload";
import json from "rollup-plugin-json";
import { terser } from "rollup-plugin-terser";
const svelteConfig = require("./svelte.config");

const production = !process.env.ROLLUP_WATCH;

export default {
  input: "src/main.js",
  output: {
    sourcemap: true,
    format: "iife",
    name: "app",
    file: "public/build/bundle.js"
  },
  moduleContext: id => {
    const thisAsWindowForModules = [
      "node_modules/intl-messageformat/lib/core.js",
      "node_modules/intl-messageformat/lib/compiler.js"
    ];

    if (thisAsWindowForModules.some(id_ => id.trimRight().endsWith(id_))) {
      return "window";
    }
  },
  plugins: [
    svelte({
      // enable run-time checks when not in production
      dev: !production,
      ...svelteConfig
    }),

    // If you have external dependencies installed from
    // npm, you'll most likely need these plugins. In
    // some cases you'll need additional configuration �
    // consult the documentation for details:
    // https://github.com/rollup/rollup-plugin-commonjs
    resolve({
      browser: true,
      dedupe: importee =>
        importee === "svelte" || importee.startsWith("svelte/")
    }),
    commonjs(),

    json({
      // for tree-shaking, properties will be declared as
      // variables, using either `var` or `const`
      preferConst: true
    }),

    // In dev mode, call `npm run start` once
    // the bundle has been generated
    !production && serve(),

    // Watch the `public` directory and refresh the
    // browser on changes when not in production
    !production && livereload("public"),

    // If we're building for production (npm run build
    // instead of npm run dev), minify
    production && terser()
  ],
  watch: {
    clearScreen: false
  }
};

function serve() {
  let started = false;

  return {
    writeBundle() {
      if (!started) {
        started = true;

        require("child_process").spawn("npm", ["run", "start", "--", "--dev"], {
          stdio: ["ignore", "inherit", "inherit"],
          shell: true
        });
      }
    }
  };
}

//svelte.config.sj
//const sveltePreprocess = require('svelte-preprocess');
//
//module.exports = {
//    // we'll extract any component CSS out into
//    // a separate file � better for performance
//    css: css => {
//        css.write('public/build/bundle.css');
//    },
//    preprocess: sveltePreprocess()
//};

My assumption is that I'm using/importing something the wrong way. Any help is appreciated!

@JohnRSim
Copy link

JohnRSim commented Jan 8, 2020

I couldn't find

      "node_modules/intl-messageformat/lib/compiler.js"

And still getting the error after updating rollup.conf

Here is my current package setup

{
  "name": "altas",
  "description": "Atlas HR Portal",
  "version": "0.0.2",
  "scripts": {
    "dev": "sapper dev",
    "build": "sapper build --legacy",
    "export": "sapper export --legacy",
    "start": "node __sapper__/build",
    "cy:run": "cypress run",
    "cy:open": "cypress open",
    "test": "run-p --race dev cy:run"
  },
  "dependencies": {
    "axios": "^0.19.0",
    "compression": "^1.7.1",
    "mobile-pull-to-refresh": "^0.2.2",
    "moment": "^2.24.0",
    "plyr": "^3.5.6",
    "polka": "^0.5.0",
    "qs": "^6.9.1",
    "sirv": "^0.4.0",
    "svelte-i18n": "^2.1.1"
  },
  "devDependencies": {
    "@babel/core": "^7.0.0",
    "@babel/plugin-syntax-dynamic-import": "^7.0.0",
    "@babel/plugin-transform-runtime": "^7.0.0",
    "@babel/preset-env": "^7.0.0",
    "@babel/runtime": "^7.0.0",
    "@rollup/plugin-json": "^4.0.0",
    "npm-run-all": "^4.1.5",
    "rollup": "^1.12.0",
    "rollup-plugin-babel": "^4.0.2",
    "rollup-plugin-commonjs": "^10.0.0",
    "rollup-plugin-node-resolve": "^5.2.0",
    "rollup-plugin-replace": "^2.2.0",
    "rollup-plugin-svelte": "^5.0.1",
    "rollup-plugin-terser": "^4.0.4",
    "sapper": "^0.27.0",
    "svelte": "^3.0.0"
  },
  "keywords": []
}

Using intl-messageformat 7.8.0
upgraded from 7.5.2

@kaisermann
Copy link
Owner

A workaround was added to the FAQ section of the wiki: https://github.com/kaisermann/svelte-i18n/wiki/FAQ#this-keyword-is-equivalent-to-undefined

It's best to add a condition to ignore this kind of warning and forget about it 😁 .

@omirobarcelo What you're getting is related to how rollup handles dynamic imports. It's not as straightforward to use them as in webpack. A starting point for you: https://github.com/rollup/rollup-starter-code-splitting.

@kaisermann kaisermann removed the help wanted Extra attention is needed label Jan 14, 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

5 participants