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

fix(build): publish Node (server) friendly bundles #3483

Merged
merged 2 commits into from
Apr 5, 2021

Conversation

laurentgoudet
Copy link
Contributor

Fixes #3279. @ionic-native libraries currently break server-side rendering as Node does not natively handle ES modules yet (well it kinda does since v13 but the current LTS is still v12 and they are many limitations, e.g. only the default CommonJS export can be imported from an ES module using import).

For that reason libraries that are published as ES modules (which is great as it provides better tree-shaking) should also provide legacy UMD or CommonJS bundles for Node to be able to natively execute the code on the server, as explained in the Angular Package Format specification, maintained by the Angular team.

Note that most of the time such code never gets executed on the server, as most @ionic-native functionality doesn't make sense in that environment, but the only fact that it uses the ES modules syntax causes the JS parser to throw a SyntaxError: Unexpected token import when reading those files.

As an aside starting with Angular CLI v9, bundleDependencies defaults to true for server-side builds which will cause the app dependencies to go through Webpack, which will transpile ES imports into CommonJS ones, effectively making @ionic-native packages work on the server. However bundleDependencies still has many limitations such as poor i18n support (angular/angular-cli#25726), and the problem remains when not using the Angular CLI for instance.

This PR fixes the issue by simply using Rollup to downgrade the previously built ES modules into bundle.js CommonJS bundles, and set those as the packages main entry points (used by require() on the server). The packages module entry points (used by client-side module bundlers, e.g. Webpack) remains unchanged.

@danielsogl
Copy link
Owner

@ihadeed please take a look if this is needed

@danielsogl danielsogl added this to the 6.0.0 milestone Aug 14, 2020
@laurentgoudet
Copy link
Contributor Author

Bump, can this be reviewed? This is still very much needed as I have to maintain my own fork of the @ionic-native packages due to that issue 😕. Thanks!

@laurentgoudet
Copy link
Contributor Author

Can I please get a review on that? Adding an extra publish format is a small change and having to maintain my own fork is quite cumbersome. Thanks!

@laurentgoudet
Copy link
Contributor Author

Bump :). I can rebase again if needed.

@laurentgoudet
Copy link
Contributor Author

@danielsogl / @ihadeed can this please be looked into? this is a fairly trivial PR - since it just publishes the bundles as an extra NodeJS friendly format, for a quite painful issue, as there's no easy way to work around it.

@danielsogl danielsogl merged commit b4227f2 into danielsogl:master Apr 5, 2021
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.

ES6 imports in dist files creates issues with server side rendering
2 participants