Skip to content

Convert to esm module syntax#606

Merged
acalcutt merged 24 commits intomaptiler:masterfrom
WifiDB:esm_update
Sep 28, 2022
Merged

Convert to esm module syntax#606
acalcutt merged 24 commits intomaptiler:masterfrom
WifiDB:esm_update

Conversation

@acalcutt
Copy link
Copy Markdown
Collaborator

@acalcutt acalcutt commented Sep 16, 2022

Updates package to esm module syntax, updates many dependencies. Replaced requires with imports.

One note relating to 'serve_light.js'. I had a lot of trouble making this so it would switch between full and light version without require. I ended up making this dummy file to load when 'serve_rendered.js' isn't needed.

Minimum node version recommended is v14.20.0 with this. node 18 still doesn't work but that seems to be a canvas issue that I image will get an update eventually.

Comment thread package.json Outdated
Comment thread src/main.js Outdated
Comment thread src/main.js
@acalcutt acalcutt changed the title Convert to esm module santax Convert to esm module syntax Sep 16, 2022
@mnutt
Copy link
Copy Markdown
Contributor

mnutt commented Sep 18, 2022

It seems like given static es6 imports, maybe the 'right' way to do the light version would be to have a separate entrypoint into the app. It'd probably be a lot to add now, but maybe in the future with some refactoring.

@acalcutt
Copy link
Copy Markdown
Collaborator Author

acalcutt commented Sep 18, 2022

I'm definitely up for suggestions on the light version. I had tried a few ideas I had found online, but this is the one I ended up getting to work. I was just happy to not have to get rid of that feature at the time. The light version basically takes a copy of the whole directory, and then switches some things off based on the package name.

To be honest I'm more a general scripting programmer than a javascript programmer, so I am always open to tips of how i can do things better.

@acalcutt
Copy link
Copy Markdown
Collaborator Author

acalcutt commented Sep 22, 2022

I'd be fine with waiting on this until after the 4.0.0 release. If we can get the package updates and readme update in, we can do this one later. the main feature is maplibre and updates to all the packages.

Comment thread src/main.js
Comment thread src/main.js
const styleFileRel = styleName + '/style.json';
const styleFile = path.resolve(styleDir, 'styles', styleFileRel);
if (fs.existsSync(styleFile)) {
config['styles'][styleName] = {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This could probably be JS instead of json.

config.styles[styleName] = {
  style: styleFileRel,
  tilejson: {
    bounds,
  },
};

Comment thread src/main.js
.replace(/:/g, '_')
.replace(/\?/g, '_')] = {
"mbtiles": path.basename(mbtilesFile)
.replace(/\//g, '_')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This may be able to be flattened into a single replace; .replace(/[\/:\?]+/g, '_') I realized this wasn't a change, just something for future reference.

Comment thread src/serve_rendered.js
Comment thread src/main.js
Comment thread src/main.js
Comment thread src/serve_data.js
import {getTileUrls, fixTileJSONCenter} from './utils.js';

module.exports = {
export const serve_data = {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe also not for this PR, but we might consider replacing all of the snake_case exports with camelCase ones.

Comment thread src/server.js
// do not require `serve_rendered` in the light package
serve_rendered = require('./serve_rendered');
}
const serve_rendered = (await import(`${!isLight ? `./serve_rendered.js` : `./serve_light.js`}`)).serve_rendered;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is actually the optimal solution for now.

The other idea I floated was to have two entrypoints (src/main.js and src/light.js or something) and main.js would import server_rendered.js while light.js would include the stub, then they each would import src/server.js and pass serve_rendered method to it. But in its current state it'd be a lot of duplication and a fair amount of passing serve_rendered around, or a global variable or something. Neither is better than what we have here, IMO.

@acalcutt acalcutt merged commit b2bd5ea into maptiler:master Sep 28, 2022
@acalcutt acalcutt mentioned this pull request Sep 28, 2022
@vinayakkulkarni
Copy link
Copy Markdown
Contributor

This is awesome !

Thanks for the updates :)

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants