Coder Social home page Coder Social logo

Comments (23)

backspaces avatar backspaces commented on July 20, 2024 2

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:

├── [email protected]
├── [email protected]
└── [email protected]

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.

from snowpack.

FredKSchott avatar FredKSchott commented on July 20, 2024 2

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

from snowpack.

krumware avatar krumware commented on July 20, 2024 2

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

  1. into package.json:
  "@pika/web": {
    "webDependencies": [
      "socket.io-client"
    ]
  }
  1. run
npx @pika/web
  1. update imports to use
import io from '/web_modules/socket.io-client.js';
  1. profit.

from snowpack.

backspaces avatar backspaces commented on July 20, 2024 1

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.

[email protected] /Users/owen/Dropbox/src/tests/cjs
├── [email protected]
├── [email protected]
├── [email protected]
└── [email protected]

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>

from snowpack.

sdegutis avatar sdegutis commented on July 20, 2024 1

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

from snowpack.

FredKSchott avatar FredKSchott commented on July 20, 2024

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?

from snowpack.

backspaces avatar backspaces commented on July 20, 2024

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.

from snowpack.

krumware avatar krumware commented on July 20, 2024

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!

from snowpack.

backspaces avatar backspaces commented on July 20, 2024

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.

from snowpack.

FredKSchott avatar FredKSchott commented on July 20, 2024

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 #68 / #49

from snowpack.

backspaces avatar backspaces commented on July 20, 2024

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.

from snowpack.

sdegutis avatar sdegutis commented on July 20, 2024

@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?

from snowpack.

sdegutis avatar sdegutis commented on July 20, 2024

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.

from snowpack.

sdegutis avatar sdegutis commented on July 20, 2024

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."

from snowpack.

sdegutis avatar sdegutis commented on July 20, 2024

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.

from snowpack.

sdegutis avatar sdegutis commented on July 20, 2024

(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.)

from snowpack.

sdegutis avatar sdegutis commented on July 20, 2024

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).

from snowpack.

sdegutis avatar sdegutis commented on July 20, 2024

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.

from snowpack.

dy avatar dy commented on July 20, 2024

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

from snowpack.

FredKSchott avatar FredKSchott commented on July 20, 2024

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:

  1. 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.
  2. 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.

from snowpack.

FredKSchott avatar FredKSchott commented on July 20, 2024

boom!

from snowpack.

bskimball avatar bskimball commented on July 20, 2024

@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

from snowpack.

krumware avatar krumware commented on July 20, 2024

@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+

from snowpack.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.