ljharb / es-abstract

ECMAScript spec abstract operations.
MIT License
114 stars 30 forks source link

[Breaking] `GetIntrinsic`: restrict intrinsic syntax #102

Closed ExE-Boss closed 10 months ago

ExE-Boss commented 4 years ago

This makes it so that the intrinsic name has to either be enclosed by %, or the enclosing %‑signs must be omitted, but it’s invalid to include only one of them.

This also disallows dynamic property access, since it’s not allowed by the spec.

ljharb commented 4 years ago

OK, so it looks like the current behavior:

I'd prefer to see this PR changed into a semver-patch to fix the first three items, and then in the future we can worry about the bracket intrinsic symbol syntax.

ExE-Boss commented 4 years ago

The code returns undefined because the !allowMissing && !(parts[i] in value) check is only performed when $gOPD is supported and i === parts.length - 1.

codecov[bot] commented 4 years ago

Codecov Report

Merging #102 into master will decrease coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #102      +/-   ##
==========================================
- Coverage   90.97%   90.97%   -0.01%     
==========================================
  Files         647      647              
  Lines        8931     8973      +42     
  Branches     2115     2124       +9     
==========================================
+ Hits         8125     8163      +38     
- Misses        806      810       +4     
Impacted Files Coverage Δ
GetIntrinsic.js 96.74% <100.00%> (+1.25%) :arrow_up:
2015/Set.js 92.30% <0.00%> (-2.70%) :arrow_down:
2016/Set.js 92.30% <0.00%> (-2.70%) :arrow_down:
2017/Set.js 92.30% <0.00%> (-2.70%) :arrow_down:
2018/Set.js 92.30% <0.00%> (-2.70%) :arrow_down:
2019/Set.js 92.30% <0.00%> (-2.70%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7a963e3...ae0833d. Read the comment docs.

ljharb commented 3 years ago

I've pulled a few of these fixes into get-intrinsic (which has now been extracted to a separate package).

If all that would remain in this PR are the breaking changes, I think we can close this; if not, it'd be great to pull any remaining bugfixes into get-intrinsic.

ljharb commented 10 months ago

Closing with thanks; if there's any further problems or changes, please file them at https://github.com/ljharb/get-intrinsic