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

Generate source maps during development only and remove from production release #6

Merged
merged 2 commits into from
May 28, 2024

Conversation

eliot-akira
Copy link
Contributor

@eliot-akira eliot-akira commented May 25, 2024

@nicolas-jaussaud I wanted to ask your opinion on this new feature.

I added a global config option (and per-task option) to generate source maps during development only (roll dev) and remove them for production (roll build).

export default {
  map: 'dev',
  build: [ ... ]
}

My motivation was to reduce the size of the plugin for L&L. I found that WordPress and Gutenberg projects are also not publishing their source maps. But ACF does.

There are arguments in support of including source maps in production release, as can be seen in this discussion:

The main advantage is that it improves debugging any JS issues that occur in production, so users can include a more helpful stack trace in the issue description. That's a pretty compelling reason, it's valuable and maybe worth a few extra megabytes.

In this case, a majority of devs preferred to not change the current default (no sourcemaps for prod), and instead added an option to enable it specifically.

From this, I was considering making it the default for Tangible Roller also.

Would you be OK with that, or would you prefer to publish source maps by default? In the latter case, I can make it opt-in and keep the current behavior.

…ng development only and remove for production
@eliot-akira eliot-akira changed the title Generate source maps during development only and remove for production Generate source maps during development only and remove from production release May 25, 2024
@nicolas-jaussaud
Copy link
Contributor

Recently I noticed that the source map was really big in fields (almost 4MB!), and I was wondering if it could be an issue

I guess it's nice to keep our plugins as small as possible for people with slow internet connections

Maybe it could also cause issues when uploading our plugins on shared hosting if there is a low file upload limit (I'm not sure what would be a usual limit for this on a shared hosting, so maybe that's not really an issue)

Personally, I wouldn't mind not having the map files by default at all because I'm familiar with our tools and will know how to switch to a dev build if needed

But it's a good point that it will be harder to report an issue for users, and even for an outside dev it will require figuring out our build process in order to generate the dev files and debug more efficiently (but we also let every file needed to do it so that could be a nice compromise?)

I'm not sure my feedback is really helpful so far haha, but I don't have a clear preference. Maybe I would be a little more in favor of removing source maps for production build, because I imagine the size difference is going to be big enough to justify it

@eliot-akira
Copy link
Contributor Author

Thanks for the feedback, that's helpful to hear your thoughts on it. The size of the Fields module was part of my motivation too, because eventually it will be included in L&L and maybe other plugins, like in an admin settings screen.

A few megabytes is usually not that big of a deal, but I noticed it slows down the initial startup time of the Playground and runnable code examples I'm working on for the docs site.

OK, so I'll change the default in this pull request, and we can see how it goes with map=dev. It will allow opt-out with the previous behavior map=true (global or per task).

@eliot-akira eliot-akira merged commit 4d64d87 into main May 28, 2024
@eliot-akira eliot-akira deleted the no-sourcemaps-for-production branch May 28, 2024 11:53
@eliot-akira
Copy link
Contributor Author

Published in version 2.0.1.

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 participants