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

Use modules everywhere #12241

Merged
merged 2 commits into from May 1, 2021
Merged

Use modules everywhere #12241

merged 2 commits into from May 1, 2021

Conversation

tschaub
Copy link
Member

@tschaub tschaub commented Apr 25, 2021

This updates our package.json to use modules everywhere.

Server-side use in Node.js

This branch makes it possible for people to use (untranspiled) modules from the ol package directly in Node. Node 12.17 and beyond can import ES modules directly without a flag. It was not possible to import or require (untranspiled) modules from the ol package previously in Node because we don't have a "type": "module" in our published package.json. The default behavior in Node is to treat packages as CommonJS packages, and we use import in our package (which doesn't work in Node with CommonJS modules).

Client-side use with a bundler

This branch doesn't change anything for people who were using import and the ol package with a bundler like Rollup, Parcel, webpack, or esbuild.

See the examples below:

It is also still possible to use require to import ol modules with webpack, Rollup, etc. I'm not sure if people are doing that, but before and after this change, you can still do things like const Map = require('ol/Map').default;.

Developing in the OpenLayers repo

There are a few things to understand about how this changes scripts and tooling within the OpenLayers repo. By adding "type": "module" to our package.json, we can no longer use require to import *.js scripts in the repo. And the same goes for other dev tooling. We can use require if we need to by using the *.cjs extension on scripts. Alternatively, we can place package.json files with "type": "commonjs" in the repo in directories where we need to author CommonJS modules instead of ES modules. One of these two strategies is required to work with tooling that doesn't yet support ES modules.

Webpack ES module support

As mentioned above, for client-side module bundling webpack works well with ES modules. However, webpack has limited support for ES modules in things like loaders or plugins (see webpack/webpack#13233). We can work around this by naming our custom loader(s) with a *.cjs extension.

Karma ES module support

Karma doesn't support authoring config files as ES modules (see karma-runner/karma#3677). As above, we can work around this by naming our config file karma.config.cjs.

JSDoc ES module support

JSDoc doesn't support authoring plugins or templates with ES modules. Because JSDoc assumes that a template will have a publish.js file, we cannot work around this by renaming our files to have a *.cjs extension. Instead, we add a package.json with "type": "commonjs" to the root of our JSDoc config directory.

Mocha ES module support

For in-browser tests, Mocha works as it did before - with the webpack transpiled modules. This branch also adds the ability to run tests directly in Node. For the Node-based tests, Mocha works with ES modules in single-run mode but not in watch mode (see mochajs/mocha#4374). The tests run fast enough in Node that I don't think this is a problem, but it will be nice when there is a solution.

I've added a test/node directory where we can place tests that are not browser-specific. For now, I've just copied in a handful of the test/browser tests. We could decide that all tests that don't do DOM manipulations should run directly in Node (this is a much simpler setup than the Karma/webpack setup). If others think this is a good idea, I could continue porting some of the existing Karma tests to run directly in Node.

You can try out the results with npm install ol@module.

Fixes #10821.
Fixes #12186.

@@ -0,0 +1 @@
16
Copy link
Member Author

Choose a reason for hiding this comment

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

I added this to configure the version of Node used in our Netlify builds. If we don't want this file in the repo, we can instead set a NODE_VERSION env var before building the site.

A potential benefit of having this file here is to set the version of Node for others that use nvm.

package.json Outdated Show resolved Hide resolved
@tschaub

This comment has been minimized.

@tschaub tschaub mentioned this pull request Apr 29, 2021
@tschaub
Copy link
Member Author

tschaub commented Apr 30, 2021

Porting over a handful of tests from test/browser to test/node, it looks like the test runtime for the Karma/webpack tests is reduced by about 15 seconds (down to about 2 minutes) and the Node test runtime is about 3 seconds.

@tschaub tschaub marked this pull request as ready for review April 30, 2021 16:36
Copy link
Member

@ahocevar ahocevar left a comment

Choose a reason for hiding this comment

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

Very impressive work @tschaub, thanks for your effort on this! I like the idea of having tests that do not rely on the DOM or other browser APIs in test/node. Moving more tests to test/node could also be done in a separate pull request and should not be blocking this, if you want to get this pull request in.

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

Successfully merging this pull request may close these issues.

Make OpenLayers Node.js friendly package.json type module
2 participants