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

package.json type module #10821

Closed
bkuster opened this issue Mar 19, 2020 · 22 comments · Fixed by #12241
Closed

package.json type module #10821

bkuster opened this issue Mar 19, 2020 · 22 comments · Fixed by #12241

Comments

@bkuster
Copy link
Contributor

bkuster commented Mar 19, 2020

Describe the bug
Using the new (but still exprimental) node esm loading mechanism does not work with the current version of ol's package.json.

To Reproduce

  1. npm i ol
  2. in index.js
    import { createXYZ } from 'ol/tilegrid.js';
  3. node --experimental-modules index.js gives the following error:

Cannot use import statement outside a module

Expected behavior
The module should be imported.

Fix
Add "type": "module" to the package.json

@ahocevar
Copy link
Member

We have evaluated this and decided to wait with adding "type": "module" until ES modules are stable in Node (perhaps Node 14). In the meantime, you can use esm to make OpenLayers work in a Node environment.

@bkuster
Copy link
Contributor Author

bkuster commented Mar 20, 2020

Thanks for the feedback. True esm works, as long as I dont write my package as a module (I think, haven't tried this yet exctensively). I decided to wait for Node 14 as well for now, had some other issues with the experimental API.

@bkuster bkuster closed this as completed Mar 20, 2020
@mmomtchev
Copy link

@ahocevar, any chances of reevaluating your decision? 😄
There is still the somewhat weird decision of the team to not enable --experimental-specifier-resolution=node - there is some kind of heated philosophical debate about the ES specifications - but hopefully they will take the right decision

@ahocevar
Copy link
Member

@tschaub may want to weigh in here, because he did some more research on the topic a while ago. I'm not opposed to adding this setting, but mostly it needs testing with the bundlers we have examples for (see https://github.com/openlayers/openlayers/#getting-started). In addition, I want to test it with some of my applications where I use OpenLayers with Node and esm. Unfortunately my schedule is quite tight at the moment, so it might be May before I get to it. So if anyone can test and provide reports here, it might speed up the process.

@ahocevar ahocevar reopened this Apr 12, 2021
@bkuster
Copy link
Contributor Author

bkuster commented Apr 12, 2021

To bud in here as the one who opened this. I am actually using openlayers in a node module where esm doesnt help me out. To get it to work, I just hacked "type": "module" into your package.json in a post install hook. So far so good 😉, works like a charm.

@mmomtchev
Copy link

@ahocevar, how about an alternative, less invasive, solution - publishing the transpiled ol.js that you use as script include either in the ol package, either as a separate package?
Then you can

import { default as ol } from 'ol/ol';

and use everything as in the examples.

The only downside is that you won't be able to share code between the server and the client-side.

@bkuster
Copy link
Contributor Author

bkuster commented Apr 14, 2021

Additional downside: you loose tree shacking, no?

@mmomtchev
Copy link

@bkuster, yes, but this is server-side, tree shaking doesn't mean much here
It is not a perfect solution, but it is one that a) solves the problem, b) doesn't change anything for everyone else

@ahocevar
Copy link
Member

I think we don't want to publish the legacy build, so we'll rather go the route of testing. I'd say to make this easier for everyone, we'd accept a pull request to make the suggested change in package.json. Then we'll have something to test with https://github.com/openlayers/ol-rollup, https://github.com/openlayers/ol-webpack, https://github.com/openlayers/ol-parcel, https://github.com/openlayers/ol-browserify, and in two yet-to-create repositories, one with node + CommonJS + esm, and one with node modules.

@mmomtchev
Copy link

You want a PR for adding "type": "module" to package.json? With a criminal affair in the background? 😄 Maybe I shall call an ambulance?

There is a reason why harassment on the workplace is a serious criminal offence, while harassment outside of it usually elicits a smile, except in the very rare cases of a criminally insane person, who cannot comprehend when and why people laugh at him.

@ahocevar
Copy link
Member

Now do you want "type": "module" or not?

@bkuster
Copy link
Contributor Author

bkuster commented Apr 16, 2021

I won't get around to it today, but my colleagues might. Otherwise I'll open a PR Monday and verify it works with your bundler examples. We've tested it with web-dev-server so far.

@tschaub
Copy link
Member

tschaub commented Apr 25, 2021

I decided to resurrect an old branch that converted our package to "type": "module" for the whole repo - not just for the published package. This wasn't working that well with Node < 14 (when I last tried it) but is working better now.

You can npm install ol@module to try it out. Here is a draft PR for the ol-parcel project: openlayers/ol-parcel#1.

There is still work to do to convert the test utilities (and probably doc things). I'll migrate #12241 out of draft state when that is done.

@bkuster
Copy link
Contributor Author

bkuster commented Apr 26, 2021

@tschaub thanks for the update and the work you've already put in. Should have some time to check it out next week.

@WORMSS
Copy link

WORMSS commented Oct 6, 2021

Anyone know what the last version of openlayers that used commonjs ?
now that it is esm, I have a large project, that now needs openlayers. It's built in typescript so I use import foo from 'ol' but then gets compiled down to commonjs and so becomes require('ol'), and this is where things break as OL is the only library that is esm. I tried to quickly turn our project into "type": "module" but it will require ALL imports to be changed to .js and that is not going to fly with my managers.

So, if anyone knows how far back I need to go to get the last "proper" commonjs version? I got as far back as 6.4.3 which states it's not a module, but as we can see above, it IS a module, but you guys failed to add 'module' as the type..

@ahocevar
Copy link
Member

ahocevar commented Oct 6, 2021

OpenLayers never published CommonJS modules. It sounds like your code runs in Node, not in a browser. If this is the case, I'd suggest using the latest ol version and - if you haven't already - upgrading to Node >= 14, or using https://npmjs.com/package/esm for older Node versions. Then ES modules should work.

@WORMSS
Copy link

WORMSS commented Oct 6, 2021

We have the latest node. But our project codebase compiles down from typescript to commonjs.
I've looked at this esm, and I don't think I can use it.. because I can't make code

requre = require("esm")(module);
module.exports = requrie("ol/foo");

in typescript I write

import ol from 'ol/foo';

and it gets turned into

var ol = __importDefault(require('ol/foo'))

in the generated files.
I have no ability to inject this esm stuff.

@ahocevar
Copy link
Member

ahocevar commented Oct 6, 2021

Looks like you're using tsc to generate that code. You could simply configure "module": "es2015", in the "compilerOptions" section of your tsconfig.json.

@WORMSS
Copy link

WORMSS commented Oct 6, 2021

Yep, and a that point, node cannot run the code unless I make package.json a "type":"module".. which puts me back to having to add the extension onto every import.

@ahocevar
Copy link
Member

ahocevar commented Oct 6, 2021

Without seeing more of your project setup, I cannot provide specific advice. But I have successfully used OpenLayer s in a CommonJS setup both natively with Node 16 and with esm in previous Node versions.

@maxicarlos08
Copy link

I am having similar issues with SvelteKit, I have changed all ol imports to .js, but now I get this strange error while building:

file://path_to_project/node_modules/ol/source/GeoTIFF.js:22
import { Pool, fromUrl as tiffFromUrl, fromUrls as tiffFromUrls } from 'geotiff';
         ^^^^
SyntaxError: Named export 'Pool' not found. The requested module 'geotiff' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'geotiff';
const { Pool, fromUrl: tiffFromUrl, fromUrls: tiffFromUrls } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:124:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:181:5)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:281:24)
    at async Object.load_component (file://path_to_project/.svelte-kit/output/server/chunks/app-444e4962.js:362:13)
    at async Promise.all (index 1)
    at async respond$1 (file://path_to_project/node_modules/@sveltejs/kit/dist/ssr.js:1211:11)
    at async render_page (file://path_to_project/node_modules/@sveltejs/kit/dist/ssr.js:1454:19)
    at async resolve (file://path_to_project/node_modules/@sveltejs/kit/dist/ssr.js:1717:10)
    at async respond (file://path_to_project/node_modules/@sveltejs/kit/dist/ssr.js:1696:10)

Should this be reported to geotiff or am I doing something wrong?

@tschaub
Copy link
Member

tschaub commented Dec 14, 2021

In case anybody comes across this, see #13114 (comment) for a working example using SvelteKit and OpenLayers. The GeoTIFF source cannot (currently) be imported in Node, but others can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants