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

Bump to v2.1.5-pre.1 - Fix postinstall.js NODE_ENV check #993

Closed
wants to merge 2 commits into from

Conversation

wipfli
Copy link
Member

@wipfli wipfli commented Feb 12, 2022

Launch Checklist

Follow-up on #991, #992. Fixes #990.

  • 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.
  • 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.
  • Manually test the debug page.
  • Suggest a changelog category: bug/feature/docs/etc. or "skip changelog".
  • Add an entry inside this element for inclusion in the maplibre-gl-js changelog: <changelog></changelog>.

@wipfli wipfli changed the title Bump to v2.1.5 - Fix postinstall.js NODE_ENV check Bump to v2.1.5-pre.1 - Fix postinstall.js NODE_ENV check Feb 12, 2022
@wipfli
Copy link
Member Author

wipfli commented Feb 12, 2022

I am playing here with getting the environment variable stuff right. But the best solution in my view would be to publish this library with an empty postinstall.js file. Then we don't have to configure anything in our dev environments and users can set NODE_ENV to whatever they want. With the current solution here, a user might set a global variable NODE_ENV='development' and then the same error as after #992 happens.

@github-actions
Copy link
Contributor

Bundle size report:

Size Change: +5 B
Total Size Before: 194 kB
Total Size After: 194 kB

Output file Before After Change
maplibre-gl.js 185 kB 185 kB +5 B
maplibre-gl.css 9.43 kB 9.43 kB 0 B
ℹ️ View Details No major changes

@HarelM
Copy link
Member

HarelM commented Feb 12, 2022

I think publishing an empty postinstall.js file is a good solution.

@HarelM
Copy link
Member

HarelM commented Feb 12, 2022

I'm not sure how this can be done easily, but the first thing that comes to my mind is doing it in the release.yml script...?

@HarelM
Copy link
Member

HarelM commented Feb 12, 2022

Another option is to remove this line from package.json just before publishing the package, again in the release.yml script, but this is probably harder as it requires to change a part of a file and not a whole file. IDK...

@wipfli
Copy link
Member Author

wipfli commented Feb 12, 2022

#994

@wipfli wipfli closed this Feb 12, 2022
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.

2.1.2 broken
2 participants