Skip to content
This repository has been archived by the owner on Jun 2, 2022. It is now read-only.

Feature/es module commonjs #339

Merged
merged 12 commits into from
Oct 27, 2020
Merged

Feature/es module commonjs #339

merged 12 commits into from
Oct 27, 2020

Conversation

ols
Copy link
Contributor

@ols ols commented Oct 26, 2020

ES module and CommonJS

Current approach is the following: Exporting dist with esm and commonjs for non-react jsx, which is logary, logary-plugin-browser, logary-plugin-node. logary-plugin-nextjs and logary-plugin-react is only using commonjs.

This means that when we run with-nextjs and with-nextjs-app we are using commonjs but when we are bundling logary-browser we are using esm with tree-shaking to get minimal bundle min file.

Here is some findings regarding es2015 usage as module rather than CommonJS

When using es2015 as module type in tsconfig.js you will get the following error:
SyntaxError: Cannot use import statement outside a module:

The recommended approach here is to use “type”: “module” defined in package.json.

But when setting type as module it will result in the following error:
Error [ERR_REQUIRE_ESM]: Must use import to load ES Module:

Here is a possible solution which does not work:

Another possible approach is to use both ES Module and CommonJS which is allowing the consumer to choose.

Some more readings in the subject:

“they are already interpreted with a conflicting opinion. TS produces a JS file that the JS code it produces does not point to. ES6 is the closest thing to an officially correct way to do JavaScript, so if the ES6 code produced by TypesScript is wrong, this is a bug plain and simple. There is no need to wait for any proprosals and such, just fix TypeScript's bug. But today people feel that if they don't find fault with something and put it through 10 layers of proposal before acting, then they are not acting intellectually. Give me a break.”

ols added 11 commits October 24, 2020 12:51
Creating a dist/* including both es2015 and commonjs since Node does not support ES modules out of the box.
…es with the es2015 (esm) version, cannot specify type: module which seems to be required to get pass module not found error, but this in return give ERR_REQUIRE_ESM and does not seem to get passed with this setup. When using only commonjs it seems to work fine.. wip…
…ame problem related to support for mjs for typeescript which do not want to do changes before they are sure the underlying libs are clear about the roadmap etc.
… from non EmacScript module.. this i will roll with commonjs for now...
…logary-plugin-browser to enable us to get three shaking for our bundle logaru.min.js since there is a problem when referencing esm from react packages.
@haf
Copy link
Member

haf commented Oct 26, 2020

+1 but squash when you merge.

browser(instance)
// A workaround for following issue:
// https://github.com/open-telemetry/opentelemetry-js/issues/1575
if (typeof document !== "undefined") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is it we don't use window which is customary?

@ols ols merged commit f112b6c into master Oct 27, 2020
@haf haf deleted the feature/es-module-commonjs branch October 27, 2020 11:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants