Convert CommonJS projects to es6 modules #62

backspaces · 2019-06-16T21:21:33Z

TLDR: This is a feature request to manage common js dependencies as well as es6 module dependencies.

Details:
If my dependencies includes a common js project, try converting it to es6 modules via https://github.com/rollup/rollup-plugin-commonjs (which is already in pika/web's package.json devDependencies).

It would be fine to limit this to explicit @pika/web.webDependencies rather than all of my package.json dependencies.

FredKSchott · 2019-06-17T06:24:45Z

I'd love to know if this is possible. My understanding was that the plugin could understand common.js dependencies, but wouldn't support common.js entrypoints (the top-level webDependency package) because it couldn't get the export interface right without knowing what was being imported off of it. Do you know if that is true?

backspaces · 2019-06-17T23:17:08Z

Hi Fred. OK, to test this, I used a mapbox module, osmtogeojson, in a new temporary repo. The repo has an npm install of the current rollup and rollup-plugin-commonjs, along with the test module osmtogeojson:

├── osmtogeojson@3.0.0-beta.3
├── rollup@1.15.6
└── rollup-plugin-commonjs@10.0.0

I initially used this with an html file using a script tag, to convert an osm (open street map) format to geojson to make sure it all works. It did.

So I then created an esm rollup via:

import commonjs from 'rollup-plugin-commonjs'

export default {
    input: './node_modules/osmtogeojson/osmtogeojson.js',
    output: {
        file: 'osmtogeojson.esm.js',
        format: 'es',
    },
    plugins: [commonjs({})],
}

I ran this, changing the script version of the html file to a <script type="module"> and used an import:

        <script type="module">
            import osmtogeojson from './osmtogeojson.esm.js'

It worked perfectly! Yay!

I've used this before with good results.

Is this what you were wondering about? Basically, many well formed modules using require/comonjs can apparently be converted to working es6 modules. I will say the sample size is small, but hopeful.

backspaces · 2019-06-19T00:17:13Z

TLDR: I tried this on another es5 repo to make sure this is a reasonable approach, and it worked.

Details: Another example: chart.js. I submitted an issue to them about modules, and in particular, showed how to create an es6 module via the rollup idea above.
chartjs/Chart.js#5179 (comment)

Just to make sure the above would still work, I created a new repo, with npm init, and installed rollup & plugins as well as chart.js.

cjs@1.0.0 /Users/owen/Dropbox/src/tests/cjs
├── chart.js@2.8.0
├── rollup@1.15.6
├── rollup-plugin-commonjs@10.0.0
└── rollup-plugin-node-resolve@5.0.3

I used this rollup.config.json:

import commonjs from 'rollup-plugin-commonjs'
import resolve from 'rollup-plugin-node-resolve'

export default {
    input: 'Chart.js',
    output: {
        file: 'chart.esm.js',
        format: 'esm',
    },
    plugins: [resolve(), commonjs()],
}

I ran rollup via npx like this npx rollup -c which produced

Chart.js → chart.esm.js...
created chart.esm.js in 1.5s

Just to make sure it worked, I made a quick chart which worked.

<!DOCTYPE html>
<html>
    <head>
        <title>Test</title>
    </head>
    <body>
        <canvas id="chart"></canvas>
        <script type="module">
            import chart from './chart.esm.js'
            const canvas = document.getElementById('chart')
            new chart(canvas, {
                type: 'line',
                data: {
                    labels: [
                        'January',
                        'February',
                        'March',
                        'April',
                        'May',
                        'June',
                        'July',
                    ],
                    datasets: [
                        {
                            label: 'My First Dataset',
                            data: [65, 59, 80, 81, 56, 55, 40],
                            fill: false,
                            borderColor: 'rgb(75, 192, 192)',
                            lineTension: 0.1,
                        },
                    ],
                },
                options: {},
            })
        </script>
    </body>
</html>

backspaces · 2019-06-19T00:22:51Z

So I think we're on to something here. I'd start with limiting this to explicit @pika/web.webDependencies rather than all of my package.json dependencies.

I don't know how to test for success. We can test for rollup errors. And maybe build a simple html file that just imports the module and signaling a failure to load somehow.

krumware · 2019-06-19T12:30:30Z

Over here crossing paths with you on chart.js. Already using chartjs in a project based on pikaweb (but the nasty old fashioned run around hacky way, per that chart.js issue). This would be amazing!

backspaces · 2019-06-19T15:45:25Z

Hi @krumware, glad you're joining the conversation. I can use a sanity check!
Everyone: the Chart.js connection is here: chartjs/Chart.js#5179 (comment)

Did you ever try the rollup which uses the rollup-plugin-commonjs? I'd love feed back.

I did not look into the potential corner cases involving Moment for example. Moment has been converted to es6, but the issue is how would that work with the chart.esm.js mentioned above.

I think the answer here is that chart.js has a rollup, dist/Chart.bundle.js, which includes Moment.

So if we point pika/web to that, via @pika/web.webDependencies, then things should work. I'll give it a try.

FredKSchott · 2019-06-21T04:29:31Z

TIL! This could be really great, thanks for doing the research on this!

Also because we use the commonjs plugin to handle internal, sub-dependencies, it looks like you can just point to common.js files (but not the package name) and @pika/web will already handle them, example:

"webDependencies": ["date-now/index.js"]

$ npx @pika/web
✔ @pika/web installed: date-now/index.js. [0.04s]

$ cat web_modules/date-now/index.js
var dateNow = now;

function now() {
    return new Date().getTime()
}

export default dateNow;
//# sourceMappingURL=index.js.map

I'm going to be on vacation for the next two weeks, and won't have regular access to internet. Which sucks timing wise, because this is really exciting. But It also sounds like there's not too much actual implementation work here, just thinking through the interface / how the user turns this on, is that correct? Open to any ideas here!

Do we just use the "main" entrypoint if "module" doesn't exist?
Do we give any heads up if we do that?
Agreed that this should start as opt-in "webDependencies" only. We are also exploring better auto-detecting via Idea for zero-configuration #68 / Feature request: infer webDependencies from imports #49

backspaces · 2019-06-21T15:51:49Z

Yay! Glad it works out, especially basically as-is. And thanks for pointing at the two other issues which merge nicely with this.

No worries about the timing, have a great, relaxing, insightful vacation.

sdegutis · 2019-06-21T15:55:37Z

@backspaces Hi, I'm from issue #68, hope you don't mind my joining this conversation. (Likewise I'd be curious about your thoughts in that issue too.)

So if I'm reading both of you correctly, the ability to bundle CJS modules such as "chart.js" and "osmtogeojson" is already available in @pika/web by just adding the main entry-point file (the same as its pkg.main) to pkg.[@pika/web].webDependencies?

sdegutis · 2019-06-21T16:55:30Z

This seems to work!

    "webDependencies": [
      "styled-components/dist/styled-components.browser.esm.js",

import styled from '/web_modules/styled-components/dist/styled-components.browser.esm.js';

Although it's verbose. Normally I would suggest an optional alternative "alias" syntax, such as:

    "webDependencies": [
      ["styled-components", "styled-components/dist/styled-components.browser.esm.js"],

Then we could just do this:

import styled from '/web_modules/styled-components.js';

However, I think the plan in #68 would solve this in a cleaner and better way: let Pika automatically scan imports in user code, finding the "/web_modules/styled-components.js" path and determining from this that it must compile the "styled-components" NPM module.

That said, Pika still wouldn't know exactly how to compile styled-components without first knowing tha it must find "dist/styled-components.browser.esm.js", which is not listed in styled-component's package.main:

  "main": "dist/styled-components.cjs.js",
  "module": "dist/styled-components.esm.js",

Those are both node-specific bundles, so styled-components may be an exception to this working smoothly. But in the general case, I think as long as Pika can find either "module" or "main" and correctly bundle it using rollup-plugin-commonjs as suggested by this issue, then this plan should work for a good majority of packages.

sdegutis · 2019-06-21T17:09:16Z

Ah, this trick of putting "styled-components/dist/styled-components.browser.esm.js" inside webDependencies doesn't work correctly for packages that import styled from 'styled-components'; directly, such as "styled-icons". As expected, @pika/web then looks for styled-components and finds "module": "dist/styled-components.esm.js" which is not the correct file.

Even though this is specific to styled-components, which was verified to be a strange exception, it's generally true about any package which doesn't have a proper pkg.module || pkg.main entry point.

I think as a reasonable workaround, users can let @pika/web know about potential "incorrect" packages like this, similar to the "alias" idea above. Maybe in conjunction with the plan in #68, instead of a whitelist, there would be an aliases entry:

  "@pika/web": {
    "aliases": {
      "styled-components": "styled-components/dist/styled-components.browser.esm.js",

Here, devs would be able to specify "real" entry points for broken packages, and Pika would be able to "alias" them in a configuration with Rollup, by saying "every time you see styled-components, either directly, or via another package which is requiring it, use this file instead as the entry-point."

sdegutis · 2019-06-21T20:45:45Z

I just verified that the concept itself works:

import commonjs from 'rollup-plugin-commonjs'
import resolve from 'rollup-plugin-node-resolve'

export default {
  input: [
    'styled-components',
    'styled-icons/fa-solid/Clipboard',
    'Chart.js',
    'react',
    'react-dom',
  ],
  output: {
    dir: 'out',
    format: 'esm',
  },
  plugins: [
    renameHardcodedTest(),
    resolve(),
    commonjs(),
  ],
};

function renameHardcodedTest() {
  return {
    name: 'rename-hardcoded-test',
    resolveId(src) {
      if (src === 'styled-components') {
        return 'node_modules/styled-components/dist/styled-components.browser.cjs.js';
      }
      return null;
    },
  };
}

This works even with the import of styled-components found nested inside of styled-icons, and the output Clipboard.esm.js file contains import styled from './styled-components.browser.cjs.js'; which is indeed the only styled-components file there is in the "out" directory.

The function renameHardcodedTest stands in place of the logic that would look at Object.entries(pkg['@pika/web'].aliases) and use that as the map.

I had to prefix the returned result with "node_modules" because otherwise Rollup wasn't able to find it. It seems that this is something the user should have to include in the aliased path themselves, because there's nothing magical about this prefix, it's just part of the path to the actual file that the user wants to include, starting at the project root.

sdegutis · 2019-06-21T20:46:47Z

(It's worth noting that the example in that comment uses @reactesm/react{,-dom} and installs it in react{,-dom}, because otherwise I couldn't get anything working with the actual React which isn't compatible with Rollup yet.)

sdegutis · 2019-06-25T17:20:28Z

One problem with this Rollup config is that the structure of the output directory doesn't match what we're importing:

out/
├── Chart.js
├── Clipboard.esm.js
├── chunk-5732a1e2.js
├── chunk-7be8ab94.js
├── react-dom.min.js
├── react.min.js
└── styled-components.browser.cjs.js

There's a few differences:

Often the filenames are different, such as the "react.js" in import '/web_modules/react.js' is different than the "react.min.js" that we actually see in the output directory.
The structure is different, such as how we have "Clipboard.esm.js" in the root of the output directory, whereas the input ("styled-icons/fa-solid/Clipboard") is gathered from a hypothetical import like import '/web_modules/styled-icons/fa-solid/Clipboard.js' which is nested.

When I add preserveModules: true, I do get a hierarchical tree, but I still don't get the structure that matches what the code hypothetically saw when scanning the imports:

out/
├── @emotion
│   ├── is-prop-valid
│   │   └── dist
│   │       └── is-prop-valid.esm.js
│   ├── memoize
│   │   └── dist
│   │       └── memoize.esm.js
│   └── unitless
│       └── dist
│           └── unitless.esm.js
├── Chart.js
│   └── dist
│       └── Chart.js
├── _virtual
│   ├── ReactPropTypesSecret.js_commonjs-proxy
│   ├── _commonjsHelpers.js
│   ├── checkPropTypes.js_commonjs-proxy
│   ├── factoryWithThrowingShims.js_commonjs-proxy
│   ├── factoryWithTypeCheckers.js_commonjs-proxy
│   ├── index.js_commonjs-proxy
│   ├── index2.js_commonjs-proxy
│   ├── index3.js_commonjs-proxy
│   ├── index4.js_commonjs-proxy
│   ├── moment.js_commonjs-proxy
│   ├── react-is.development.js_commonjs-proxy
│   ├── react-is.production.min.js_commonjs-proxy
│   ├── styled-components.browser.cjs.js
│   └── stylis.min.js_commonjs-proxy
├── is-what
│   └── dist
│       └── index.esm.js
├── memoize-one
│   └── dist
│       └── memoize-one.esm.js
├── merge-anything
│   └── dist
│       └── index.esm.js
├── moment
│   └── moment.js
├── object-assign
│   └── index.js
├── prop-types
│   ├── checkPropTypes.js
│   ├── factoryWithThrowingShims.js
│   ├── factoryWithTypeCheckers.js
│   ├── index.js
│   └── lib
│       └── ReactPropTypesSecret.js
├── react
│   └── react.min.js
├── react-dom
│   └── react-dom.min.js
├── react-is
│   ├── cjs
│   │   ├── react-is.development.js
│   │   └── react-is.production.min.js
│   └── index.js
├── styled-icons
│   ├── StyledIconBase
│   │   └── StyledIconBase.esm.js
│   ├── fa-solid
│   │   └── Clipboard
│   │       └── Clipboard.esm.js
│   └── node_modules
│       └── @emotion
│           └── is-prop-valid
│               └── dist
│                   └── is-prop-valid.esm.js
├── stylis
│   └── stylis.min.js
├── stylis-rule-sheet
│   └── index.js
└── tslib
    └── tslib.es6.js

Now we have "out/_virtual/styled-components.browser.cjs.js" instead of just "out/styled-components.browser.cjs.js"
We also still have "styled-icons/fa-solid/Clipboard/Clipboard.esm.js" which has the wrong filename (and ideally would be one directory higher but that point is confusing and slightly unrelated to this).

sdegutis · 2019-06-25T18:12:12Z

Regarding that last problem, Pika/web already seems to have the ability to do this, because it outputs modules matching the file/directory structure of webDependencies. But I can't for the life of me figure out where in the code this is happening. No output options seem relevant to it, none of the 5 plugins seem to make it happen. I'm just confused.

sdegutis · 2019-06-25T21:40:31Z

Got that working!

Turns out I needed to turn the array into a map:

  input: {
    'styled-components': 'styled-components',
    'styled-icons/fa-solid/Clipboard': 'styled-icons/fa-solid/Clipboard',
    'Chart.js': 'Chart.js',
    'react': 'react',
    'react-dom': 'react-dom',
  },

out
├── Chart.js.js
├── chunk-5732a1e2.js
├── chunk-7be8ab94.js
├── react-dom.js
├── react.js
├── styled-components.js
└── styled-icons
    └── fa-solid
        └── Clipboard.js

dy · 2019-06-30T01:31:31Z

FYI https://github.com/brechtcs/webrunify

FredKSchott · 2019-07-15T20:35:44Z

hey all! I'm back from vacation and really excited about this feature. I think there are two paths to go down that would make sense here:

Quick fix: For packages whitelisted in webDependences, support fallbacks for when the "module" entrypoint is not found. Basically, check for "module", then "browser", then finally "main" with a backwards-compat CJS. This will get rid of some of the verbosity problems around our current CJS backwards-compat story.
Larger fix: I think we want to move forward (carefully) with support for auto-detecting/analyzing imports, which could also help with this issue.

What do people thing? I'm still catching up on issues/PRs, so apologies if I'm missing anything that's already been said outside of this thread.

FredKSchott · 2019-07-26T02:10:40Z

Update: the quick-fix (supporting CJS packages in the "webDependencies" whitelist) has been merged to master. It won't be released for a few more days. But, you can try it now via:

npm install http://cdn.pika.dev/@pika/web/master

krumware · 2019-08-05T15:52:35Z

This immediately helped with socket.io-client, which is a classic case of a frustratingly almost-compatible browser module.

into package.json:

  "@pika/web": {
    "webDependencies": [
      "socket.io-client"
    ]
  }

run

npx @pika/web

update imports to use

import io from '/web_modules/socket.io-client.js';

profit.

FredKSchott · 2019-08-05T17:28:07Z

bskimball · 2020-11-11T11:32:38Z

@krumware Are you still able to use socket.io-client with this set up? I can't seem to get it to work

Edit: I got it to work.

Instead of
import io from "socket.io-client"
do this
import io from "socket.io-client/dist/socket.io.js"

I'm also using chart.js so that import looks like:
import Chart from "chart.js/dist/Chart.js
as opposed to using rollup to create an esm module

krumware · 2020-11-13T22:44:54Z

@bskimball sorry for the late reply. I haven't used it in some time, but your solution makes sense since importing the full file path is required.

Also, Chart.js has a Next branch with esm support, unless they've already released 3.0+

backspaces mentioned this issue Jun 18, 2019

Why not main entry #65

Closed

backspaces mentioned this issue Jun 19, 2019

Chart.js does not import as es6 module chartjs/Chart.js#5179

Closed

sdegutis mentioned this issue Jun 21, 2019

Idea for zero-configuration #68

Closed

sdegutis mentioned this issue Jun 24, 2019

Make this package work with tools that use ESM Stanko/react-animate-height#73

Merged

sdegutis mentioned this issue Jun 25, 2019

Cache busting and package versioning #69

Closed

FredKSchott mentioned this issue Jul 15, 2019

Automatically convert "not modern" packages in Pika CDN FredKSchott/pika-pack#62

Closed

FredKSchott mentioned this issue Jul 26, 2019

default to main? #44

Closed

FredKSchott closed this as completed Aug 5, 2019

cliqer mentioned this issue Jul 2, 2020

Question: How to load Socket.io-client and other libraries that only support require? vitejs/vite#490

Closed

drwpow pushed a commit that referenced this issue Jul 24, 2020

add react fast refresh (#62)

62042ad

drwpow pushed a commit that referenced this issue Jul 27, 2020

add react fast refresh (#62)

5267cdf

drwpow pushed a commit that referenced this issue Jul 27, 2020

add react fast refresh (#62)

31d0a1a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert CommonJS projects to es6 modules #62

Convert CommonJS projects to es6 modules #62

backspaces commented Jun 16, 2019

FredKSchott commented Jun 17, 2019

backspaces commented Jun 17, 2019

backspaces commented Jun 19, 2019

backspaces commented Jun 19, 2019

krumware commented Jun 19, 2019 •

edited

Loading

backspaces commented Jun 19, 2019

FredKSchott commented Jun 21, 2019

backspaces commented Jun 21, 2019

sdegutis commented Jun 21, 2019 •

edited

Loading

sdegutis commented Jun 21, 2019

sdegutis commented Jun 21, 2019

sdegutis commented Jun 21, 2019

sdegutis commented Jun 21, 2019

sdegutis commented Jun 25, 2019

sdegutis commented Jun 25, 2019

sdegutis commented Jun 25, 2019

dy commented Jun 30, 2019

FredKSchott commented Jul 15, 2019 •

edited

Loading

FredKSchott commented Jul 26, 2019

krumware commented Aug 5, 2019

FredKSchott commented Aug 5, 2019

bskimball commented Nov 11, 2020 •

edited

Loading

krumware commented Nov 13, 2020

Convert CommonJS projects to es6 modules #62

Convert CommonJS projects to es6 modules #62

Comments

backspaces commented Jun 16, 2019

FredKSchott commented Jun 17, 2019

backspaces commented Jun 17, 2019

backspaces commented Jun 19, 2019

backspaces commented Jun 19, 2019

krumware commented Jun 19, 2019 • edited Loading

backspaces commented Jun 19, 2019

FredKSchott commented Jun 21, 2019

backspaces commented Jun 21, 2019

sdegutis commented Jun 21, 2019 • edited Loading

sdegutis commented Jun 21, 2019

sdegutis commented Jun 21, 2019

sdegutis commented Jun 21, 2019

sdegutis commented Jun 21, 2019

sdegutis commented Jun 25, 2019

sdegutis commented Jun 25, 2019

sdegutis commented Jun 25, 2019

dy commented Jun 30, 2019

FredKSchott commented Jul 15, 2019 • edited Loading

FredKSchott commented Jul 26, 2019

krumware commented Aug 5, 2019

FredKSchott commented Aug 5, 2019

bskimball commented Nov 11, 2020 • edited Loading

krumware commented Nov 13, 2020

krumware commented Jun 19, 2019 •

edited

Loading

sdegutis commented Jun 21, 2019 •

edited

Loading

FredKSchott commented Jul 15, 2019 •

edited

Loading

bskimball commented Nov 11, 2020 •

edited

Loading