search

netlify / zip-it-and-ship-it

Intelligently prepare Node.js Lambda functions for deployment
https://www.npmjs.com/package/@netlify/zip-it-and-ship-it
MIT License
316 stars 35 forks source link

fix(exports parser): handle `as default` export #1658

Closed lilnasy closed 1 year ago

lilnasy commented 1 year ago

Summary

The implementation may be done better, I don't have the full context of the codebase. Feel free to improve upon it.

Skn0tt commented 1 year ago

Hey @lilnasy, thanks for opening this PR - much appreciated! Coincidentally, I opened https://github.com/netlify/zip-it-and-ship-it/pull/1657 on friday which might address the same thing. I'll compare & review them sometime today.

Skn0tt commented 1 year ago

Yup, your fix is pretty much the same as mine from Friday :D I've added the code from your screenshot as a test case to https://github.com/netlify/zip-it-and-ship-it/pull/1657/commits/7dfba88d56515afc1dd29a44b50f56a4cf6a3c86. Going to close your PR, and merge mine - will ping here again once it's deployed.

lilnasy commented 1 year ago

@Skn0tt Thanks for that! Do you guys have stats on how many users are on older versions of netlify-cli? I would probably have to workaround the issue with regex. Is there a way to explicitly inform the cli of the runtime version?

Skn0tt commented 1 year ago

will ping here again once it's deployed.

The update went out to the CLI in https://github.com/netlify/cli/releases/tag/v17.5.3 and was deployed to our CI build system a couple minutes ago.

Is there a way to explicitly inform the cli of the runtime version?

I'm not aware of a way, nor did I find something in the code :/ This might be something we could build, will look into it.

Do you guys have stats on how many users are on older versions of netlify-cli?

Just ran the numbers for you. It seems like in the past 24hrs of netlify deploy executions, only about 1/3 of them were on recent versions (17.5.x), and others were lagging behind quite a bit. But it should be pretty straight-forward for most to update their CLI version.

If the regex is straight-forward for you, then that might be the best bet. If it isn't, maybe you could try to detect the version of the Netlify CLI that's being used to build, and warn/error on versions below 17.5.3? Let me know if you want to go that way, happy to help in how to detect the CLI version.

lilnasy commented 1 year ago

It would be relatively reliable and easy to string replace. I will do that.

This might be something we could build, will look into it

Yes, it will help in a similar situation in the future.

Skn0tt commented 1 year ago

Yes, it will help in a similar situation in the future.

Opened an internal issue about it, let's see when we get to it 🤷

Skn0tt commented 10 months ago

@lilnasy it's now possible to detect the version of the Netlify CLI that's running using the NETLIFY_CLI_VERSION environment variable: https://github.com/netlify/cli/pull/6226