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

Fix style-spec scripts to work in esm package/fix help #1971

Merged
merged 22 commits into from
Dec 23, 2022

Conversation

acalcutt
Copy link
Contributor

@acalcutt acalcutt commented Dec 21, 2022

Attempt to fix #1963

This PR updates the syntax of the style-spec bin scripts to be ESM syntax and adds a file extension to fix the following error
TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension "" for /root/.nvm/versions/node/v16.15.0/lib/node_modules/@maplibre/maplibre-gl-style-spec/bin/gl-style-migrate. Loading extensionless files is not supported inside of "type":"module" package.json contexts. The package.json file /root/.nvm/versions/node/v16.15.0/lib/node_modules/@maplibre/maplibre-gl-style-spec/package.json caused this "type":"module" context. Try changing /root/.nvm/versions/node/v16.15.0/lib/node_modules/@maplibre/maplibre-gl-style-spec/bin/gl-style-migrate to have a file extension. Note the "bin" field of package.json can point to a file with an extension, for example {"type":"module","bin":{"gl-style-migrate":"./bin/gl-style-migrate.js"}}

it also adds missing help information for 'gl-style-migrate' which did not exist before.

It also removes 'gl-style-composite' for #1981

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
  • Briefly describe the changes in this PR.
  • Link to related issues.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality.
  • Document any changes to public APIs.
  • Post benchmark scores.
  • Add an entry to CHANGELOG.md under the ## main section.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 21, 2022

Bundle size report:

Size Change: 0 B
Total Size Before: 207 kB
Total Size After: 207 kB

Output file Before After Change
maplibre-gl.js 197 kB 197 kB 0 B
maplibre-gl.css 9.1 kB 9.1 kB 0 B
ℹ️ View Details No major changes

@HarelM
Copy link
Collaborator

HarelM commented Dec 21, 2022

The type in the package.json file of the style-spec is defined as module which assumes js files are esm now and not cjs.
I would recommend changing the scripts to use import instead of require and not keep them in common js format.
If this avoids the need to rename them, I think it's a better option...

@acalcutt
Copy link
Contributor Author

I mentioned changing these extensions to cjs is probably the wrong approach in the issue thread, which is why this is still a draft. This works but those scripts should be easy to convert since they are small

I was planning to attempt to change it to ESM syntax next, I just wanted to submit what I got working last night before I went to bed.

@HarelM
Copy link
Collaborator

HarelM commented Dec 21, 2022

Sorry, didn't mean to jump too soon. Thanks for looking into this!

@acalcutt acalcutt marked this pull request as ready for review December 22, 2022 02:04
@acalcutt
Copy link
Contributor Author

I've tested this with
npm i -g @acalcutt/maplibre-gl-style-spec@17.0.7

and basic-v7.json as a test file

@zstadler
Copy link
Contributor

I've tested all scripts with a few of styles here. They all work flawlessly.

@HarelM
Copy link
Collaborator

HarelM commented Dec 22, 2022

Thanks!
Can you please add a changelog item in both style-spec and the main one and bump the version of style-spec so that we can release a new version of it?

@acalcutt
Copy link
Contributor Author

I added the changelogs and updated the version

CHANGELOG.md Outdated Show resolved Hide resolved
@HarelM HarelM merged commit 9546a91 into maplibre:main Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Errors running the MapLibre GL style utilities
3 participants