feat: add config option to prevent import into client code - #12480
feat: add config option to prevent import into client code#12480Dudek-AMS wants to merge 8 commits into
Conversation
…ing imported to client site code
|
|
Ive dropped the changeset so far to be added by maintainers. |
|
After reconsidering the naming, I would suggest naming the attribute serverOnlyPaths |
Dudek-AMS
changed the title
fix import paths removed unused imports
| import { app_server, env_dynamic_private, env_static_private } from '../module_ids.js'; | ||
| import { load_config } from '../../../core/config/index.js'; | ||
|
|
||
| const svelte_config = await load_config(); |
There was a problem hiding this comment.
loading the config again could be a breaking change in case a config contains one time setup code that is then executed twice.
There was a problem hiding this comment.
Yeah mentioned it above, didnt saw another way to get the config on this part of code
There was a problem hiding this comment.
that would have to be found then. using top-level-await here is also not great.
| if (configRule.test(id)) return true; | ||
| } else if (typeof configRule === 'function') { | ||
| const check = configRule(id); | ||
| if (typeof check === 'boolean') return check; //always return the boolean to allow exceptions, if its undefined continue to allow multiple rules |
There was a problem hiding this comment.
early return should only happen on true, otherwise a return false could give access to an id that would be denied by a later check
There was a problem hiding this comment.
that would be a design decision. If I want to make a exception and allow a file import, no matter what other rules say, the early return is needed. but depends on execution order then. If a rule dont care, it returns undefined/nothing
There was a problem hiding this comment.
this would allow for conflicting rules and a "first one wins" scenario, more complex, even harder to understand and not currently documented in the jsdoc in this PR.
There was a problem hiding this comment.
if really reducing to only one rule and not allowing exceptions i'd drop anything but regex, should be flexible enough IMO
|
3 different ways to define user-supplied rules seems a bit much, esp. if there is ()=>boolean already, users can just use regex or string compare themselves. But before continuing on this PR please see my comment in #12477 |
the idea was to make the function method for more complicated rules, while string is prefered. ofc function only does work out |
teemingc
added
needs-decision
teemingc
changed the title
|
closing per #12477 (comment) |
5 participants
Closes #12477
Added
serverProtectedPathsserverOnlyPaths to add additional rules to prevent import into client codePlease don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits