patriciogonzalezvivo/glslCanvas

"Error: invalid module" in Observable

riccardoscalco opened this issue · 14 comments

Hi,

the following code in https://beta.observablehq.com/ returns "Error: invalid module":

glslCanvas = require('glslCanvas')

It worked until few days ago.

Adding the release it works as expected:

glslCanvas = require('glslCanvas@0.0.25')

Hi @riccardoscalco, thanks for reaching out.

The past days a big commit had landed. Some external collaborator made an important new feature. Because of that I'm jumping the version number to prevent discontinuations like this one.

Current version now is https://github.com/patriciogonzalezvivo/glslCanvas/releases/tag/0.1.3
If you can give it a try and tell me how it goes I will appreciate it.

Sorry @riccardoscalco for the inconvenience this bring!

@riccardoscalco I can help you debug but I can't figure out how to use https://beta.observablehq.com/. (Looking now)

The project builds an es version now, and perhaps observable hq is using that when you're not in es mode. That's just a guess from the error message. If you can figure out what file is being used, that'd help. Or maybe you can switch your require to an import, just another guess

For more info on the new builds, here is the PR where I updated it: #19

@giannif thanks for stepping in, I was just going to send you a message. Thanks again both!

@patriciogonzalezvivo @giannif thanks for the help.

The project builds an es version now, and perhaps observable hq is using that when you're not in es mode. That's just a guess from the error message.

I have the same feeling, consider that

glslCanvas = require('https://rawgit.com/patriciogonzalezvivo/glslCanvas/master/dist/GlslCanvas.js')

works, and

glslCanvas = require('https://rawgit.com/patriciogonzalezvivo/glslCanvas/master/dist/GlslCanvas.es.js')

does not.

You can play with an example here: https://beta.observablehq.com/@riccardoscalco/value-and-gradient-noise-in-glsl.

The error happens near the end of the page (see the image), you can change the code and then press the arrow on the right.

screen shot 2018-06-14 at 23 04 30

If you think this is an issue related to Observable, I am happy to open an issue there.

An update: as described in https://beta.observablehq.com/@tmcw/introduction-to-require, es modules should be imported with import(glslCanvas).

Unfortunately, a new error appears: "SyntaxError: The requested module 'https://unpkg.com/xhr@^2.4.0?module' does not provide an export named 'default'
"

Which sounds like an issue with xhr :)

@riccardoscalco thanks for digging in.

Looks like xhr has defined a default in the latest build:
naugtur/xhr@39ded17

So we can use that.

I've been looking into using a browser field for the package.json instead of the main, it seemed like the right answer, but now when I compile with Webpack, it uses that field instead of the module field resulting in larger builds.

Perhaps we change the main to point to the umd bundle for the browser, which is dist/GlslCanvas.js

This works for me:

{ let glslCanvas = require('glslCanvas/dist/GlslCanvas.js') }

@riccardoscalco @patriciogonzalezvivo I'm going to create a PR for what I mention above.

Before my changes, the main field pointed to the umd browser build, not a cjs build. I added the module field so we can use an es build with Webpack and Rollup, and that remains, so this is the best of both worlds

Thanks @giannif, now glslCanvas = require('glslCanvas') works.

@giannif thanks so much again!

@giannif I gave you a wrong feedback sorry, strangely this morning I have the original error..
@patriciogonzalezvivo I think it should be enough to npm publish the latest update.

Thanks again to both!

Dang! my fault sorry! Publishing 0.1.4

@patriciogonzalezvivo yes, I think it works.

I published a test page in observable https://beta.observablehq.com/@riccardoscalco/test with different imports.

import('glslCanvas') still does not work with the same error, but I am not sure if it was expected to work.

Thanks!

I'm surprised there's still an issue with the xhr import but that change would need to come from their package.

Everything we can do here has been done