feat: Added ability to import code from ee folder #18841
Conversation
|
Cool 🙌 I guess we need a test that shows something works when ee is available and one when the ee folder is deleted? We tried that with Python and the tests don't run with the ee folder deleted (even though the app will run 🫠) |
|
Size Change: 0 B Total Size: 1.83 MB ℹ️ View Unchanged
|
| @@ -0,0 +1,5 @@ | |||
| // NOTE: All exported items from the EE module _must_ be optionally defined to ensure we work well with FOSS | |||
| export type PostHogEE = { | |||
There was a problem hiding this comment.
this is really neat though! one or more types in here that define expectation and then a FOSS and EE licensed version.
Is it just typescript magic that ensure the EE version overwrites the FOSS version?
I.e. imagine the case where we want code present in both versions but different behavoiur? Which might simply be something we explicitly say we don't support (or don't support this way...)
There was a problem hiding this comment.
We will cross that bridge when we come to it tbh.
Typescript will import in order of the files mentioned which is how it works. So if the EE folder isn't present it will fallback to the base folder.
ee/frontend/exports.ts
Outdated
There was a problem hiding this comment.
Ah, we'll need the paths filter for front end CI updated to watch for changes in this folder!
There was a problem hiding this comment.
and maybe the reverse too. changes to the ee/frontend folder shouldn't trigger the backend tests
There was a problem hiding this comment.
switched to approved in case you want to merge this and then follow-up with CI changes... easy either way
I don't believe we do as it's covered the same way as all the other EE stuff. @timgl any thoughts? |
|
Ah, yeah, because the code is in the EE folder which is already licensed 🤦 Maybe a pass over the docs to make sure we're not saying "python code in the EE folder" or something but even that would only be misleading rather than break the license |
Checked with the big dogs and it is all a-okay 👍 |
|
|
Ah, you'll need to update required jobs in GitHub... Easiest thing is probably to remove jest from required tests without adding new ones. (or just do all of that fast enough that it doesn't matter what order 🤣) |
|
hmmm dorny paths filter doesn't support excluding paths... let's not worry about infrequently triggering the backend CI for now |
Problem
We want to experiment with some conditional licensing of frontend code via the
eefolder.Changes
ee/frontend/folder that allows to export via single point👉 Stay up-to-date with PostHog coding conventions for a smoother review.
How did you test this code?