feat: add conditional load of version

[zekth]

Fix: #2603

as discussed in #2604

edit: switching to conditional loading like mentioned in : #2605 (comment)

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[kibertoad]

Shouldn't we also add this to prepublish hook?

[zekth]

I'm not aware of the publish stack. Can add if required.

[kibertoad]

https://github.com/fastify/releasify is usually used.
Since it eventually does npm publish, I think standard npm prepublish hooks should work fine. prepublishOnly seems like the right one.

[jsumners]

I am 👎 on this as well. I don't think we should have to accommodate tools that modify source code they do not own. We ship a package.json with our library. We should be able to work with that package.json at run time. Reading the original issue, it sounds like the tooling the reporter uses removes that package.json and that's not our fault.

[kibertoad]

@jsumners Original issue seems to be a valid used-case, though. Webpack is a popular solution for packaging serverless functions, and omitting package.json doesn't seem to be wrong in context. Do you think that skipping version check for when package.json is not found would be a better approach?

[jsumners]

I think we clearly state that we do not support such scenarios:

fastify/docs/Serverless.md

Lines 12 to 20 in 341bbc8

	### Attention Readers:
	> Fastify is not designed to run on serverless environments.
	The Fastify framework is designed to make implementing a traditional HTTP/S server easy.
	Serverless environments requests differently than a standard HTTP/S server;
	thus, we cannot guarantee it will work as expected with Fastify.
	Regardless, based on the examples given in this document,
	it is possible to use Fastify in a serverless environment.
	Again, keep in mind that this is not Fastify's intended use case and
	we do not test for such integration scenarios.

[mcollina]

I think the best solution is to skip the check if the file could not be loaded.

[zekth]

I think the best solution is to skip the check if the file could not be loaded.

In this case on the non packaged version the test stack will work and the built version would skip it indeed. So here https://github.com/fastify/fastify/blob/master/lib/pluginUtils.js#L89 we would just have to check if this.version is defined.

So if we're unable to load the version we set version to null instead of undefined to know if we tried to load it once?

[mcollina]

Il move that check in the module scope of loadVersion() instead., and keep the value of version to undefined.

[mcollina]

Would you mind to add a test for this? You might need to use proxyquire to mock fs.

[mcollina]

https://nodejs.org/api/fs.html#fs_fs_accesssync_path_mode < let's use this instead

[agramian]

Using fs.accessSync requires wrapping the block in a try/catch since that function throws an error on failure rather than returning a boolean like fs.existsSync.

Even addressing that, this change does not resolve the issue. I just tried with this change and the following error is outputted:

fastify not found, proceeding anyway
FastifyError [Error]: fastify-plugin: fastify-compress - expected '3.x' fastify version, '%s' is installed
    at Object.checkVersion (webpack-internal:///./node_modules/fastify/lib/pluginUtils.js:96:63)
    at Object.registerPlugin (webpack-internal:///./node_modules/fastify/lib/pluginUtils.js:110:16)
    at Boot.override (webpack-internal:///./node_modules/fastify/lib/pluginOverride.js:28:57)
    at Plugin.exec (webpack-internal:///./node_modules/avvio/plugin.js:79:33)
    at Boot.loadPlugin (webpack-internal:///./node_modules/avvio/plugin.js:266:10)
    at Task.release (webpack-internal:///./node_modules/fastq/queue.js:140:16)
    at worked (webpack-internal:///./node_modules/fastq/queue.js:182:10)
    at eval (webpack-internal:///./node_modules/avvio/plugin.js:269:7)
    at done (webpack-internal:///./node_modules/avvio/plugin.js:201:5)
    at check (webpack-internal:///./node_modules/avvio/plugin.js:225:9)
    at internal/process/task_queues.js:153:7
    at AsyncResource.runInAsyncScope (async_hooks.js:186:9)
    at AsyncResource.runMicrotask (internal/process/task_queues.js:150:8)
    at processTicksAndRejections (internal/process/task_queues.js:97:5) {
  code: 'FST_ERR_PLUGIN_VERSION_MISMATCH',
  statusCode: 500
}

[mcollina]

lgtm

[jsumners]

LGTM

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.