Although what you have described is not a pattern I’ve seen employed commonly, I think architecturally there’s nothing wrong with it and I’ve seen several organizations follow a pattern like this with success. (One of them was me, in a previous job.)
However, usually when I think of “data-only module” I don’t think about there being resource blocks inside it. That makes the module inherently stateful – those resources will persist from one plan/apply round to the next – which isn’t necessarily a problem but does require a little more care, such as making sure to preserve those stateful objects between rounds should you decide to refactor the module in future.
However, modern Terraform has facilities such as moved blocks to help with that, so I don’t think it’s a big deal in practice. It just means that if you make any changes to the module in future that would cause those resources to change addresses then you would need to add moved blocks too so that Terraform will automatically migrate the existing state objects to the new addresses.