kaisermann / svelte-i18n

Internationalization library for Svelte
MIT License
1.26k stars 80 forks source link

intl-messageformat throwing warnings #12

Closed Sureiya closed 4 years ago

Sureiya commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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: https://github.com/rollup/rollup/issues/794#issuecomment-270803587. A custom onwarn handle which ignores this kind of warning.

omirobarcelo commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.