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

Import of 'intl' package fails in production mode #1925

Closed
nilshelmig opened this issue Oct 10, 2019 · 6 comments
Closed

Import of 'intl' package fails in production mode #1925

nilshelmig opened this issue Oct 10, 2019 · 6 comments

Comments

@nilshelmig
Copy link

Description

We use the material-ui (0.20.1) datepicker in some old applications. For internationalization we use the Intl namespace. Since we need support for Internet Explorer 11, we use a polyfill package intl.
Since Fable 2.3.24 we get the following error when building with webpack in production mode.

Uncaught ReferenceError: IntlPolyfill is not defined

Repro code

We use the component like this

type RCom = ReactElementType<obj>
let DatePicker = importDefault<RCom> "material-ui/DatePicker"

let datepicker props children =
  let intl = Fable.Core.JsInterop.importDefault<obj> "intl"
  importAll "intl/locale-data/jsonp/de-DE.js" |> ignore
  let props = JS.Object.assign (createObj [ "DateTimeFormat" ==> intl?DateTimeFormat
                                            "locale" ==> "de-DE"
                                            "firstDayOfWeek" ==> 1 ], props |> wrapProps)
  ReactElementType.create DatePicker props children

Expected and actual results

Expected
No runtime exception.

Actual
Runtime exception on page load

Uncaught ReferenceError: IntlPolyfill is not defined

Related information

  • Fable version: fable-compiler@2.3.24 (up to latest)
  • Operating system: Windows 10
@alfonsogarciacaro
Copy link
Member

Thanks for the report! Hmm, there were some changes in the optimization of bindings in production mode in 2.3.24. It may happen that the importAll ... |> ignore statement is being erased by the production optimizations. I'll try to reproduce it.

In the meanwhile can you please try the following instead? This is the recommended way for importing only for side effects:

importSideEffects "intl/locale-data/jsonp/de-DE.js"

@nilshelmig
Copy link
Author

I've made a minimal example to reproduce: https://github.com/nilshelmig/fable-bug

It seems that the bug only occurs when calling Fable from JavaScript. When I use Fable-bug.fsproj as an entry point, the program runs fine

@alfonsogarciacaro
Copy link
Member

Ah, ok, thanks! Then I'm pretty sure what's the problem now. When you use a import expression, even if you put it inside a function, Fable will move it to the top of the translated JS file (because you can only have imports at the file level in JS). Because of this, if you're calling Fable from JS, in production mode, tree shaking will take place and remove the import "intl/locale-data/jsonp/de-DE.js" statement because it's not actually used by the calling file.

To avoid this, this kind of polyfill imports should always way in the main file (the entry point) of your app. Another alternative is to add the polyfills directly to the entry option of your Webpack config:

module.exports = {
    entry: ["intl/locale-data/jsonp/de-DE.js", "./src/myApp.js"],
    // ...
}

@nilshelmig
Copy link
Author

I solved this issue by removing the polyfill. It seems that Intl is supported by our targeted browsers.

But I can't find an API to use Intl, so I had to use

let [<Global>] private Intl : obj = jsNative
let datepicker props children =
  let props = JS.Object.assign (createObj [ "DateTimeFormat" ==> Intl?DateTimeFormat
                                            "locale" ==> "de-DE"
                                            "firstDayOfWeek" ==> 1 ], props |> wrapProps)
  ReactElementType.create DatePicker props children

Any idea to make this cleaner?

@alfonsogarciacaro
Copy link
Member

Yes! You can take the Intl module from the old Fable.Import.Browser and contribute it as a package in the new Browser bindings :)

@nilshelmig
Copy link
Author

Will do that :)
I'm closing this issue now, since my problem is solved. Thank you ❤️

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

2 participants