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

Statically evaluate constants referenced by macros #9487

Merged
merged 8 commits into from
Jan 21, 2024
Merged

Conversation

devongovett
Copy link
Member

@devongovett devongovett commented Jan 13, 2024

This PR enables macro calls to include statically analyzable constants. For example:

import { foo } from './macro' with {type: 'macro'};

const common = {
  foo: 'bar'
};

const result1 = foo({...common, bar: 'test'});
const result2 = foo({...common, bar: 'other'});

Constants must be declared with const to ensure that they cannot be mutated, as this would be much harder to analyze. Object and array patterns are supported in the declaration in addition to simple identifiers. The result of a macro may also be stored in a const variable, and later used in another macro call.

This also implements support for member expressions and optional chaining to access properties of statically known objects and arrays.

It may be helpful to review with whitespace disabled FYI.

@mischnic
Copy link
Member

Constants must be declared with const to ensure that they cannot be mutated, as this would be much harder to analyze.

Is this also handled?

const common = {
  foo: 'bar'
};
common.foo = 'baz'; // <---
const result1 = foo({...common, bar: 'test'});

@devongovett
Copy link
Member Author

@mischnic I was just thinking about that as well. Added some code to make assignments to constant objects an error if that constant is later accessed by a macro. In addition, if a constant object (or a property of a constant object that is also an object) is passed as an argument of a function call, it is also marked as potentially mutated.


const object2 = {foo: bar, obj: {}};
doSomething(object2.obj); // error (object could be mutated)
output2 = hashString(object2);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above could potentially be fine if object2.obj is not accessed in a macro, e.g. if object2.foo were passed here. But tracking mutations for individual values would be a bit more complicated than only tracking it for constants as a whole.

@devongovett devongovett merged commit e21b186 into v2 Jan 21, 2024
12 of 16 checks passed
lettertwo added a commit that referenced this pull request Jan 30, 2024
* upstream/v2: (22 commits)
  Add source map support to the inline-require optimizer (#9511)
  [Web Extension] Add content script world property to manifest schema validation (#9510)
  feat: add getCurrentPackageManager (#9505)
  Default Bundler Contributor Notes (#9488)
  rename parentAsset to root for msb config and remove unstable (#9486)
  Macro errors -> v2 (#9501)
  Statically evaluate constants referenced by macros (#9487)
  Multiple css bundles in Entry bundle groups issue (#9023)
  Fix macro issues (#9485)
  Bump follow-redirects from 1.14.7 to 1.15.4 (#9475)
  Revert more CI changes to centos job (#9472)
  Use lightningcss to implement CSS packager (#8492)
  Fixup CI again (#9471)
  Clippy and use napi's Either3 (#9047)
  Upgrade to eslint 8 (#8580)
  Add support for JS macros (#9299)
  Fixup REPL CI (#9467)
  Drop per-pipeline transformation cache (#9459)
  Upgrade some CI actions (#9466)
  REPL (#9365)
  ...
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