Skip to content

Bump version of wasm-feature-detect - #288

Closed
sbc100 wants to merge 1 commit into
masterfrom
bump_version
Closed

sbc100 wants to merge 1 commit into
masterfrom
bump_version

Conversation

@sbc100

@sbc100 sbc100 commented Oct 3, 2022

Copy link
Copy Markdown
Member

I'm hoping to get extended-const support but there are other new detectors too I think.

I'm hoping to get extended-const support but there are other new
detectors too I think.
@sbc100
sbc100 requested review from RReverser and dschuff October 3, 2022 14:49
Comment thread roadmap.js
function _loadTooltipModule() {
// Be sure to change the preloads in markdown when updating url.
// The ESM bundle of this package doesn't work with unpkg.com.
const module = import('https://cdn.jsdelivr.net/npm/@floating-ui/dom@1/+esm');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

does the "1" here mean "1.0" (i.e. pinned to an older revision) or does it mean "get me the latest 1.x version"?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No idea.. I just verified that the 1 didn't give me the new version that I wanted and 1.3 did.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@RReverser is this the correctly thing to do here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

does the "1" here mean "1.0" (i.e. pinned to an older revision) or does it mean "get me the latest 1.x version"?

I honestly don't know; before #283 we used unpkg.com where it wouldn't be pinned, but I'm not very familiar with what jsdelivr does.

Explicitly bumping to 1.3 sounds good.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wait, I don't think this is a module you meant to bump at all. This is not wasm-feature-detect, it's just a tooltip library.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ooops! Wrong URL.

Actually I noticed that the existing URL seems to fetch 1.3 already: https://cdn.jsdelivr.net/npm/wasm-feature-detect@1/dist/esm/index.js

So I'm closing this..

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just to clarify @1 will load the latest 1.x.x version https://github.com/jsdelivr/jsdelivr#npm
The HTTP headers also verify it:

x-jsd-version: 1.3.0
x-jsd-version-type: version

@RReverser

Copy link
Copy Markdown
Member

Bumped the correct library in 2d5bcb9 instead.

@RReverser RReverser closed this Oct 4, 2022
@sbc100
sbc100 deleted the bump_version branch October 4, 2022 00:14
@sbc100

sbc100 commented Oct 4, 2022

Copy link
Copy Markdown
Member Author

Ooop, sorry for the noise. It seem that maybe this wan't needed?

@sbc100

sbc100 commented Oct 4, 2022

Copy link
Copy Markdown
Member Author

Anyway looks like the new detector for extended const is online and working as intended!

@RReverser

Copy link
Copy Markdown
Member

I think it still was - it's a bit scary because @1 seems to resolve to whatever's latest 1.x.y is; that is, it seems likely that between 1.3 release of wasm-feature-detect and your PR fixing the feature name #286 we were briefly broken due to incompatible feature names.

Better to be semver-explicit.

@RReverser

Copy link
Copy Markdown
Member

Anyway looks like the new detector for extended const is online and working as intended!

Are there browsers where it works for you? If it's available under a flag in some browsers already, we should reflect that in features.json.

@RReverser

Copy link
Copy Markdown
Member

Looks like we did have it before: https://github.com/WebAssembly/website/blob/master/features.json#L97

Here and in other browsers the feature needs to be renamed as well now.

@sbc100

sbc100 commented Oct 4, 2022

Copy link
Copy Markdown
Member Author

Yes, it works in chrome with flag, I added that in #285, but it looks like I need another update because I called it extendedConstantExprs instead of extendedConst. Sending another PR now.

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.

4 participants